From beeaa9bc45464cf85776d7f80d9e9413ab0ebe48 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Fri, 4 Mar 2011 12:47:30 +1100
Subject: [PATCH] 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>
---
 segments.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/segments.c b/segments.c
index 63f7df8..d422a84 100644
--- 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;
-- 
2.39.5