bug fixes: revalidate dentries passed to all inode/super operations

Be sure to properly revalidate all dentry chains passed to all inode and
super_block operations.  Remove the older BUG_ON test is_valid_dentry().
This should help improve cache-coherency.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index a7ca35e88e4230c69fc917ca62471b944ed7b513..de50e76f1a7652961b29c2b76c0f44dc6bc8772b 100644 (file)
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -47,7 +47,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
        valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd);
        unionfs_unlock_dentry(dentry->d_parent);
        if (!valid) {
-               err = -ENOENT;  /* same as what real_lookup does */
+               err = -ESTALE;  /* same as what real_lookup does */
                goto out;
        }
        valid = __unionfs_d_revalidate_chain(dentry, nd);
@@ -252,6 +252,12 @@ static struct dentry *unionfs_lookup(struct inode *parent,
        struct path path_save;
        struct dentry *ret;
 
+       /*
+        * lookup is the only special function which takes a dentry, yet we
+        * do NOT want to call __unionfs_d_revalidate_chain because by
+        * definition, we don't have a valid dentry here yet.
+        */
+
        /* save the dentry & vfsmnt from namei */
        if (nd) {
                path_save.dentry = nd->dentry;
@@ -286,11 +292,18 @@ static int unionfs_link(struct dentry *old_dentry, struct inode *dir,
        struct dentry *whiteout_dentry;
        char *name = NULL;
 
-       BUG_ON(!is_valid_dentry(new_dentry));
-       BUG_ON(!is_valid_dentry(old_dentry));
-
        unionfs_double_lock_dentry(new_dentry, old_dentry);
 
+       if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+       if (new_dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(new_dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_new_dentry = unionfs_lower_dentry(new_dentry);
 
        /*
@@ -424,10 +437,14 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
        int bindex = 0, bstart;
        char *name = NULL;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
 
+       if (dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        /* We start out in the leftmost branch. */
        bstart = dbstart(dentry);
 
@@ -578,9 +595,14 @@ static int unionfs_mkdir(struct inode *parent, struct dentry *dentry, int mode)
        int whiteout_unlinked = 0;
        struct sioq_args args;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        bstart = dbstart(dentry);
 
        hidden_dentry = unionfs_lower_dentry(dentry);
@@ -719,9 +741,14 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
        char *name = NULL;
        int whiteout_unlinked = 0;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        bstart = dbstart(dentry);
 
        hidden_dentry = unionfs_lower_dentry(dentry);
@@ -836,9 +863,13 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
        int err;
        struct dentry *hidden_dentry;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_dentry = unionfs_lower_dentry(dentry);
 
        if (!hidden_dentry->d_inode->i_op ||
@@ -866,7 +897,13 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
        int len = PAGE_SIZE, err;
        mm_segment_t old_fs;
 
-       BUG_ON(!is_valid_dentry(dentry));
+       unionfs_lock_dentry(dentry);
+
+       if (dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(dentry, nd)) {
+               err = -ESTALE;
+               goto out;
+       }
 
        /* This is freed by the put_link method assuming a successful call. */
        buf = kmalloc(len, GFP_KERNEL);
@@ -890,6 +927,7 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
        err = 0;
 
 out:
+       unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return ERR_PTR(err);
 }
@@ -897,6 +935,11 @@ out:
 static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
                             void *cookie)
 {
+       unionfs_lock_dentry(dentry);
+       if (!__unionfs_d_revalidate_chain(dentry, nd))
+               printk("unionfs: put_link failed to revalidate dentry\n");
+       unionfs_unlock_dentry(dentry);
+
        unionfs_check_dentry(dentry);
        kfree(nd_get_link(nd));
 }
@@ -1052,9 +1095,13 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
        int i;
        int copyup = 0;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        bstart = dbstart(dentry);
        bend = dbend(dentry);
        inode = dentry->d_inode;

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 38fd7ff4e5704323de6d0c7d34089c1a62672557..3f6366a9983a25f54fc0023c296a08a68480ddb7 100644 (file)
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -401,11 +401,18 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
        int err = 0;
        struct dentry *wh_dentry;
 
-       BUG_ON(!is_valid_dentry(old_dentry));
-       BUG_ON(!is_valid_dentry(new_dentry));
-
        unionfs_double_lock_dentry(old_dentry, new_dentry);
 
+       if (!__unionfs_d_revalidate_chain(old_dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+       if (!d_deleted(new_dentry) && new_dentry->d_inode &&
+           !__unionfs_d_revalidate_chain(new_dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        if (!S_ISDIR(old_dentry->d_inode->i_mode))
                err = unionfs_partial_lookup(old_dentry);
        else

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 86d6327955d60c1ac6293cc5215b2e3ffd71c1ed..0a12a168189f16b7c47148c1e675361dc92407ba 100644 (file)
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -117,8 +117,13 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
        struct super_block *sb;
        struct dentry *lower_dentry;
 
-       BUG_ON(!is_valid_dentry(dentry));
        unionfs_check_dentry(dentry);
+       unionfs_lock_dentry(dentry);
+
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
 
        sb = dentry->d_sb;
 
@@ -144,6 +149,8 @@ static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
        memset(&buf->f_fsid, 0, sizeof(__kernel_fsid_t));
        memset(&buf->f_spare, 0, sizeof(buf->f_spare));
 
+out:
+       unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return err;
 }

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index d2aadb8c4d0112eeac528c20b4e5eaf33feab238..3709e57cdf43f5e12ce01a0e0e342dcd7a583cd0 100644 (file)
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -401,19 +401,6 @@ static inline int is_robranch(const struct dentry *dentry)
        return is_robranch_idx(dentry, index);
 }
 
-/*
- * Check if dentry is valid or not, as per our generation numbers.
- * @dentry: dentry to check.
- * Returns 1 (valid) or 0 (invalid/stale).
- */
-static inline int is_valid_dentry(struct dentry *dentry)
-{
-       BUG_ON(!UNIONFS_D(dentry));
-       BUG_ON(!UNIONFS_SB(dentry->d_sb));
-       return (atomic_read(&UNIONFS_D(dentry)->generation) ==
-               atomic_read(&UNIONFS_SB(dentry->d_sb)->generation));
-}
-
 /* What do we use for whiteouts. */
 #define UNIONFS_WHPFX ".wh."
 #define UNIONFS_WHLEN 4

diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index b3d814d384e74247455f3c0b24798443cdc93234..69233cccf62a7485b8114cba7ca70f065ca2dde0 100644 (file)
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -73,11 +73,14 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 {
        int err = 0;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_check_dentry(dentry);
        unionfs_lock_dentry(dentry);
 
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        err = unionfs_unlink_whiteout(dir, dentry);
        /* call d_drop so the system "forgets" about us */
        if (!err) {
@@ -87,6 +90,7 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
                d_drop(dentry);
        }
 
+out:
        unionfs_unlock_dentry(dentry);
        return err;
 }
@@ -128,11 +132,14 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
        int err = 0;
        struct unionfs_dir_state *namelist = NULL;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_check_dentry(dentry);
        unionfs_lock_dentry(dentry);
 
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        /* check if this unionfs directory is empty or not */
        err = check_empty(dentry, &namelist);
        if (err)

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 4cacead60c61a7d7053832068b231b5b386fdb9a..bf8f1cfec0da2f8e6493c4f87cdde8c3c27ef8b3 100644 (file)
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -57,14 +57,18 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
        struct dentry *hidden_dentry = NULL;
        int err = -EOPNOTSUPP;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
 
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_dentry = unionfs_lower_dentry(dentry);
 
        err = vfs_getxattr(hidden_dentry, (char*) name, value, size);
 
+out:
        unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return err;
@@ -80,14 +84,19 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
        struct dentry *hidden_dentry = NULL;
        int err = -EOPNOTSUPP;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_dentry = unionfs_lower_dentry(dentry);
 
        err = vfs_setxattr(hidden_dentry, (char*) name, (void*) value,
                           size, flags);
 
+out:
        unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return err;
@@ -102,13 +111,18 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
        struct dentry *hidden_dentry = NULL;
        int err = -EOPNOTSUPP;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
+
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_dentry = unionfs_lower_dentry(dentry);
 
        err = vfs_removexattr(hidden_dentry, (char*) name);
 
+out:
        unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return err;
@@ -124,15 +138,19 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
        int err = -EOPNOTSUPP;
        char *encoded_list = NULL;
 
-       BUG_ON(!is_valid_dentry(dentry));
-
        unionfs_lock_dentry(dentry);
 
+       if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
+               err = -ESTALE;
+               goto out;
+       }
+
        hidden_dentry = unionfs_lower_dentry(dentry);
 
        encoded_list = list;
        err = vfs_listxattr(hidden_dentry, encoded_list, size);
 
+out:
        unionfs_unlock_dentry(dentry);
        unionfs_check_dentry(dentry);
        return err;