[PATCH 21/21] Unionfs: Correctly decrement refcounts of mnt's upon branch management

[Josef 'Jeff' Sipek]

From: Erez Zadok <ezk@xxxxxxxxxxxxx>

The old logic was broken in one place, which another place tried to "fix"
incorrectly.  Also added detailed comments to explain the new/correct logic.

Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@xxxxxxxxxxxxx>
---
 fs/unionfs/commonfops.c |   17 ++++++++++++-----
 fs/unionfs/dentry.c     |   18 +++++++++++++-----
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 69d9c87..83001aa 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -135,14 +135,21 @@ static void cleanup_file(struct file *file)
 				       "file %p\n", file);
 			else {
 				unionfs_read_lock(sb);
+				/* decrement count of open files */
 				branchput(sb, i);
 				unionfs_read_unlock(sb);
-				/* XXX: is it OK to use sb->s_root here? */
-				unionfs_mntput(sb->s_root, i);
-				/* mntget b/c fput below will call mntput */
-				unionfs_mntget(sb->s_root, bindex);
+				/*
+				 * fput will perform an mntput for us on the
+				 * correct branch.  Although we're using the
+				 * file's old branch configuration, bindex,
+				 * which is the old index, correctly points
+				 * to the right branch in the file's branch
+				 * list.  In other words, we're going to
+				 * mntput the correct branch even if
+				 * branches have been added/removed.
+				 */
+				fput(unionfs_lower_file_idx(file, bindex));
 			}
-			fput(unionfs_lower_file_idx(file, bindex));
 		}
 	}
 
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 95cea3b..db0ef43 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -258,14 +258,22 @@ out_this:
 	/* finally, lock this dentry and revalidate it */
 	verify_locked(dentry);
 	dgen = atomic_read(&UNIONFS_D(dentry)->generation);
-	saved_bstart = dbstart(dentry);
-	saved_bend = dbend(dentry);
 	valid = __unionfs_d_revalidate_one(dentry, nd);
 
-	if (valid && chain_len > 0 && sbgen != dgen) {
-		for (bindex = saved_bstart; bindex <= saved_bend; bindex++)
+	/*
+	 * If __unionfs_d_revalidate_one() succeeded above, then it will
+	 * have incremented the refcnt of the mnt's, but also the branch
+	 * indices of the dentry will have been updated (to take into
+	 * account any branch insertions/deletion.  So the current
+	 * dbstart/dbend match the current, and new, indices of the mnts
+	 * which __unionfs_d_revalidate_one has incremented.  Note: the "if"
+	 * test below does not depend on whether chain_len was 0 or greater.
+	 */
+	if (valid && sbgen != dgen)
+		for (bindex = dbstart(dentry);
+		     bindex <= dbend(dentry);
+		     bindex++)
 			unionfs_mntput(dentry, bindex);
-	}
 
 out_free:
 	/* unlock/dput all dentries in chain and return status */
-- 
1.5.2.rc1.165.gaf9b

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/