Hold an extra block reference in lafs_seg_ref_block.

See the comment in the code to understand why.

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

diff --git a/segments.c b/segments.c
index 63f7df85db09d451f0bfe4b36966ae7885ff2f80..d422a8492d97354c7fd1e078dafe2be21f739270 100644 (file)
--- a/segments.c
+++ b/segments.c
@@ -279,13 +279,30 @@ int lafs_seg_ref_block(struct block *b, int ssnum)
 
        while (!test_bit(B_SegRef, &b->flags)) {
                struct block *p;
-               struct block *b2;
+               struct block *b2, *to_put = NULL;
                struct segsum *ss;
                struct inode *ino;
                int err;
 
                BUG_ON(test_bit(B_Root, &b->flags));
 
+               /* The extra reference of 'b2' and the use for 'to_put'
+                * deserves some explanation.
+                * We will normally hold an implicit reference to b2 due to
+                * our reference on 'b' and the fact that b2 is an ancestor
+                * of 'b'.  However at this point we have no locks on the file
+                * at all so it is possible that a btree manipulation could
+                * split b2 resulting in b2 not being the parent of b any more
+                * so our reference guarantee is lost.  Even in that case,
+                * the ancestor of b would probably have a B_PrimaryRef on
+                * b2.  However it is hard to prove that will stay around long
+                * enough so we take an extra ref just in case.
+                * We only need the ref when we drop private_lock so only take
+                * it then.  We cannot drop and old ref at that point, so
+                * store the old ref in 'to_put' and drop it when releasing
+                * private_lock.
+                */
+
                b2 = b;
                /* Need to check parents */
                ino = b->inode;
@@ -296,36 +313,36 @@ int lafs_seg_ref_block(struct block *b, int ssnum)
                        if (test_bit(B_InoIdx, &p->flags)) {
                                struct datablock *db = LAFSI(p->inode)->dblock;
                                p = &db->b;
+                               getref(b2, MKREF(segref2));
                                spin_unlock(&ino->i_data.private_lock);
                                ino = p->inode;
+                               if (to_put)
+                                       putref(to_put, MKREF(segref2));
                                spin_lock(&ino->i_data.private_lock);
+                               to_put = b2;
                        }
                        if (test_bit(B_SegRef, &p->flags))
                                break;
                        if (!test_bit(B_Root, &p->flags))
                                b2 = p;
                }
+               getref(b2, MKREF(segref2));
                spin_unlock(&ino->i_data.private_lock);
+               if (to_put)
+                       putref(to_put, MKREF(segref2));
                /* b2 is the first ancestor (closest to root) without SegRef */
-               /* FIXME we have an implicit reference on b2
-                * through b->parent...
-                * But if a split happens, b2 might not be a
-                * true parent any more, so we no longer have a
-                * reference.  If that can actually happen,
-                * we need to take a reference before dropping
-                * the lock.
-                * If it cannot, then we don't need the lock.
-                */
 
                if (b2->physaddr == 0) {
                        /* There is no segsum to reference */
                        set_bit(B_SegRef, &b2->flags);
+                       putref(b2, MKREF(segref2));
                        continue;
                }
 
                ss = segsum_byaddr(fs, b2->physaddr, ssnum);
                if (IS_ERR(ss)) {
                        putref(b, MKREF(segref));
+                       putref(b2, MKREF(segref2));
                        return PTR_ERR(ss);
                }
 
@@ -335,6 +352,7 @@ int lafs_seg_ref_block(struct block *b, int ssnum)
                         */
                        if (test_and_set_bit(B_SegRef, &b2->flags))
                                ss_put(ss, fs);
+                       putref(b2, MKREF(segref2));
                        continue;
                }
 
@@ -344,6 +362,7 @@ int lafs_seg_ref_block(struct block *b, int ssnum)
                if (err) {
                        ss_put(ss, fs);
                        putref(b, MKREF(segref));
+                       putref(b2, MKREF(segref2));
                        return err;
                }
                if (!test_bit(B_SegRef, &ss->ssblk->b.flags)) {
@@ -352,6 +371,7 @@ int lafs_seg_ref_block(struct block *b, int ssnum)
                        ss_put(ss, fs);
                } else if (test_and_set_bit(B_SegRef, &b2->flags))
                        ss_put(ss, fs);
+               putref(b2, MKREF(segref2));
        }
        putref(b, MKREF(segref));
        return 0;