Add proper locking and refcounting to pin_all_children.

We need private_lock to walk the list,
and we don't want to refile a block that might not have
a reference count.

And only pin Dirty children.  Others don't need it.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/index.c b/index.c
index 283291e6d2a699c13a2e9348a4429fd1de33be4b..0607988fd996417ae9404b856e2ba8d49cb0ac4e 100644 (file)
--- a/index.c
+++ b/index.c
@@ -425,31 +425,63 @@ void lafs_pin_block_ph(struct block *b, int ph)
 
 static void pin_all_children(struct indexblock *ib)
 {
-       struct indexblock *p;
+       /* If we cannot get new credits are phase_flip, we need to
+        * pin all dirty data-block children so that they get
+        * written and don't remain to require a phase_flip
+        * on the next checkpoint (at which point there will be
+        * no credit left
+        * So if any dirty child is found, set phase.  We don't
+        * need PinPending as it is already Dirty.  All other 
+        * requirement for pinning will already be met.
+        */
+       struct block *b;
+       struct indexblock *start = NULL;
        int depth = 0;
        struct fs *fs = fs_from_inode(ib->b.inode);
        int ph = fs->phase;
 
-recurse:
-       p = ib;
-       ib = list_entry(&p->children, struct indexblock, b.siblings);
-loop:
-       list_for_each_entry_continue(ib, &p->children, b.siblings) {
-               if (test_bit(B_Index, &ib->b.flags)) {
-                       /* recurse down */
+       /* Locking:
+        * This is called with an IOLock on ib,  So it is unlikely
+        * that children will be added or removed(?), but we really
+        * should hole the inode private_lock anyway.
+        */
+       getiref(ib, MKREF(pinall));
+       b = NULL;
+restart:
+       /* We hold a reference on ib, and on 'start' if non-NULL */
+       spin_lock(&ib->b.inode->i_data.private_lock);
+       b = list_prepare_entry(b, &ib->children, siblings);
+       list_for_each_entry_continue(b, &ib->children, siblings) {
+               if (test_bit(B_Index, &b->flags)) {
+                       /* restart down */
                        depth++;
-                       goto recurse;
-               } else {
-                       set_phase(&ib->b, ph);
-                       lafs_refile(&ib->b, 0);
+                       getref_locked(b, MKREF(pinall));
+                       spin_unlock(&ib->b.inode->i_data.private_lock);
+                       putiref(ib, MKREF(pinall));
+                       ib = iblk(b);
+                       b = NULL;
+                       goto restart;
+               } else if (test_bit(B_Dirty, &b->flags) &&
+                          !test_bit(B_Pinned, &b->flags)) {
+                       getref_locked(b, MKREF(pinall));
+                       spin_unlock(&ib->b.inode->i_data.private_lock);
+                       set_phase(b, ph);
+                       putref(b, MKREF(pinall));
+                       /* Slightly safer to restart this level */
+                       b = NULL;
+                       goto restart;
                }
        }
+       spin_unlock(&ib->b.inode->i_data.private_lock);
+       putiref(start, MKREF(pinall));
        if (depth) {
                depth--;
-               ib = p;
-               p = ib->b.parent;
-               goto loop;
+               start = ib;
+               ib = getiref(ib->b.parent, MKREF(pinall));
+               b = &start->b;
+               goto restart;
        }
+       putiref(ib, MKREF(pinall));
 }
 
 static void flip_phase(struct block *b)