Unionfs: unionfs_lookup locking consistency
authorErez_Zadok <ezk@cs.sunysb.edu>
Tue, 11 Sep 2007 00:15:37 +0000 (20:15 -0400)
committerErez Zadok <ezk@cs.sunysb.edu>
Fri, 29 Apr 2011 02:24:39 +0000 (22:24 -0400)
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@cs.sunysb.edu>
fs/unionfs/inode.c
fs/unionfs/lookup.c

index 687b9a70176500ac84a7de06d6b55881759726a0..9638b64aed384ba8c1ce97b80a362cdc5a7801f4 100644 (file)
@@ -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);
index 7fa631046edbba66174bde26e19e330c36b781d8..8eb97494fdcbe22ca008f6d060230809760fed10 100644 (file)
@@ -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;