[PATCH 5/6] Unionfs: unionfs_lookup locking consistency

[Erez Zadok <ezk@xxxxxxxxxxxxx>]

Ensure that our lookup locking is consistent and symmetric: if a lock
existed before calling lookup_backend, it should remain so; only if
performing a lookup of a known new dentry, should lookup_backend return a
newly-locked dentry-inode info (and only if there was no error).  Document
this behavior.  This cleanup allowed us to remove two unnecessary int
declarations.

Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
Acked-by: Josef Sipek <jsipek@xxxxxxxxxxxxxxxxx>
---
 fs/unionfs/inode.c  |    6 +++++-
 fs/unionfs/lookup.c |   38 +++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 687b9a7..9638b64 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -163,7 +163,10 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 		path_save.mnt = nd->mnt;
 	}
 
-	/* The locking is done by unionfs_lookup_backend. */
+	/*
+	 * unionfs_lookup_backend returns a locked dentry upon success,
+	 * so we'll have to unlock it below.
+	 */
 	ret = unionfs_lookup_backend(dentry, nd, INTERPOSE_LOOKUP);
 
 	/* restore the dentry & vfsmnt in namei */
@@ -176,6 +179,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 			dentry = ret;
 		/* parent times may have changed */
 		unionfs_copy_attr_times(dentry->d_parent->d_inode);
+		unionfs_unlock_dentry(dentry);
 	}
 
 	unionfs_check_inode(parent);
diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index 7fa6310..8eb9749 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -77,6 +77,11 @@ out:
  *
  * Returns: NULL (ok), ERR_PTR if an error occurred, or a non-null non-error
  * PTR if d_splice returned a different dentry.
+ *
+ * If lookupmode is INTERPOSE_PARTIAL/REVAL/REVAL_NEG, the passed dentry's
+ * inode info must be locked.  If lookupmode is INTERPOSE_LOOKUP (i.e., a
+ * newly looked-up dentry), then unionfs_lookup_backend will return a locked
+ * dentry's info, which the caller must unlock.
  */
 struct dentry *unionfs_lookup_backend(struct dentry *dentry,
 				      struct nameidata *nd, int lookupmode)
@@ -94,8 +99,6 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
 	struct dentry *first_lower_dentry = NULL;
 	struct vfsmount *first_lower_mnt = NULL;
 	int locked_parent = 0;
-	int locked_child = 0;
-	int allocated_new_info = 0;
 	int opaque;
 	char *whname = NULL;
 	const char *name;
@@ -109,24 +112,21 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
 	if (lookupmode == INTERPOSE_PARTIAL || lookupmode == INTERPOSE_REVAL ||
 	    lookupmode == INTERPOSE_REVAL_NEG)
 		verify_locked(dentry);
-	else {
+	else			/* this could only be INTERPOSE_LOOKUP */
 		BUG_ON(UNIONFS_D(dentry) != NULL);
-		locked_child = 1;
-	}
 
-	switch(lookupmode) {
-		case INTERPOSE_PARTIAL:
-			break;
-		case INTERPOSE_LOOKUP:
-			if ((err = new_dentry_private_data(dentry)))
-				goto out;
-			allocated_new_info = 1;
-			break;
-		default:
-			if ((err = realloc_dentry_private_data(dentry)))
-				goto out;
-			allocated_new_info = 1;
-			break;
+	switch (lookupmode) {
+	case INTERPOSE_PARTIAL:
+		break;
+	case INTERPOSE_LOOKUP:
+		if ((err = new_dentry_private_data(dentry)))
+			goto out;
+		break;
+	default:
+		/* default: can only be INTERPOSE_REVAL/REVAL_NEG */
+		if ((err = realloc_dentry_private_data(dentry)))
+			goto out;
+		break;
 	}
 
 	/* must initialize dentry operations */
@@ -419,7 +419,7 @@ out:
 	if (locked_parent)
 		unionfs_unlock_dentry(parent_dentry);
 	dput(parent_dentry);
-	if (locked_child || (err && allocated_new_info))
+	if (err && (lookupmode == INTERPOSE_LOOKUP))
 		unionfs_unlock_dentry(dentry);
 	if (!err && d_interposed)
 		return d_interposed;
-- 
1.5.2.2

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