Verify and maintain fanout invariants.

This somewhat long patch calls various invariant-checking (debugging)
functions in all places where the fanout invariants should hold.  The three
invariant-checking functions, __unionfs_check_{inode,dentry,file}, perform
exhaustive sanity checking on the fan-out of various Unionfs objects.  We
check that no lower objects exist outside the start/end branch range; that
all objects within are non-NULL (with some allowed exceptions); that for
every lower file there's a lower dentry+inode; that the start/end ranges
match for all corresponding lower objects; that open files/symlinks have
only one lower objects, but directories can have several; and more.

The rest of this patch actually fixes many places where these invariants did
not hold, which could lead to bugs or corruptions under heavy loads,
multi-threaded workloads, dynamic branch-management, and mmap operations.
Most of the bugs related to actions involving copyups and whiteouts.  With
these fixes, the entire Unionfs regression suite passes without a single
invariant violated.

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 2cbf5618243e560396b6c7c113c23161afb23998..d824eaa085d8a53434c05d3210383f1b72621094 100644 (file)
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -78,11 +78,18 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,
 
        /* bring it to the same state as an unlinked file */
        hidden_dentry = unionfs_lower_dentry_idx(dentry, dbstart(dentry));
+       if (!unionfs_lower_inode_idx(dentry->d_inode, bindex)) {
+               atomic_inc(&hidden_dentry->d_inode->i_count);
+               unionfs_set_lower_inode_idx(dentry->d_inode, bindex,
+                                           hidden_dentry->d_inode);
+       }
        hidden_dir_dentry = lock_parent(hidden_dentry);
        err = vfs_unlink(hidden_dir_dentry->d_inode, hidden_dentry);
        unlock_dir(hidden_dir_dentry);
 
 out:
+       if (!err)
+               unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -257,6 +264,8 @@ static int do_delayed_copyup(struct file *file, struct dentry *dentry)
 
        BUG_ON(!S_ISREG(file->f_dentry->d_inode->i_mode));
 
+       unionfs_check_file(file);
+       unionfs_check_dentry(dentry);
        for (bindex = bstart - 1; bindex >= 0; bindex--) {
                if (!d_deleted(file->f_dentry))
                        err = copyup_file(parent_inode, file, bstart,
@@ -278,9 +287,25 @@ static int do_delayed_copyup(struct file *file, struct dentry *dentry)
                                fput(unionfs_lower_file_idx(file, bindex));
                                unionfs_set_lower_file_idx(file, bindex, NULL);
                        }
+                       if (unionfs_lower_mnt_idx(dentry, bindex)) {
+                               unionfs_mntput(dentry, bindex);
+                               unionfs_set_lower_mnt_idx(dentry, bindex, NULL);
+                       }
+                       if (unionfs_lower_dentry_idx(dentry, bindex)) {
+                               BUG_ON(!dentry->d_inode);
+                               iput(unionfs_lower_inode_idx(dentry->d_inode, bindex));
+                               unionfs_set_lower_inode_idx(dentry->d_inode, bindex, NULL);
+                               dput(unionfs_lower_dentry_idx(dentry, bindex));
+                               unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
+                       }
                }
-               fbend(file) = bend;
+               /* for reg file, we only open it "once" */
+               fbend(file) = fbstart(file);
+               set_dbend(dentry, dbstart(dentry));
+               ibend(dentry->d_inode) = ibstart(dentry->d_inode);
        }
+       unionfs_check_file(file);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -374,6 +399,8 @@ out:
                kfree(UNIONFS_F(file)->saved_branch_ids);
        }
 out_nofree:
+       if (!err)
+               unionfs_check_file(file);
        unionfs_unlock_dentry(dentry);
        return err;
 }
@@ -556,6 +583,8 @@ out:
        }
 out_nofree:
        unionfs_read_unlock(inode->i_sb);
+       unionfs_check_inode(inode);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -565,10 +594,19 @@ int unionfs_file_release(struct inode *inode, struct file *file)
        struct file *hidden_file = NULL;
        struct unionfs_file_info *fileinfo = UNIONFS_F(file);
        struct unionfs_inode_info *inodeinfo = UNIONFS_I(inode);
+       struct super_block *sb = inode->i_sb;
        int bindex, bstart, bend;
-       int fgen;
+       int fgen, err = 0;
 
-       unionfs_read_lock(inode->i_sb);
+       unionfs_check_file(file);
+       unionfs_read_lock(sb);
+       /*
+        * Yes, we have to revalidate this file even if it's being released.
+        * This is important for open-but-unlinked files, as well as mmap
+        * support.
+        */
+       if ((err = unionfs_file_revalidate(file, 1)))
+               goto out;
        /* fput all the hidden files */
        fgen = atomic_read(&fileinfo->generation);
        bstart = fbstart(file);
@@ -579,9 +617,9 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 
                if (hidden_file) {
                        fput(hidden_file);
-                       unionfs_read_lock(inode->i_sb);
-                       branchput(inode->i_sb, bindex);
-                       unionfs_read_unlock(inode->i_sb);
+                       unionfs_read_lock(sb);
+                       branchput(sb, bindex);
+                       unionfs_read_unlock(sb);
                }
        }
        kfree(fileinfo->lower_files);
@@ -603,8 +641,10 @@ int unionfs_file_release(struct inode *inode, struct file *file)
                fileinfo->rdstate = NULL;
        }
        kfree(fileinfo);
-       unionfs_read_unlock(inode->i_sb);
-       return 0;
+
+out:
+       unionfs_read_unlock(sb);
+       return err;
 }
 
 /* pass the ioctl to the lower fs */
@@ -705,6 +745,7 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
        }
 
 out:
+       unionfs_check_file(file);
        return err;
 }
 
@@ -719,6 +760,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 
        if ((err = unionfs_file_revalidate(file, 1)))
                goto out;
+       unionfs_check_file(file);
 
        if (!atomic_dec_and_test(&UNIONFS_I(dentry->d_inode)->totalopens))
                goto out;
@@ -750,5 +792,6 @@ out_lock:
        unionfs_unlock_dentry(dentry);
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 2cf1878237a1a3e5989ef8c7056394f6f48ab0c4..c37c90febcf4375ced6dbbd29848b1d25dbaba0e 100644 (file)
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -483,6 +483,25 @@ out_free:
                dput(old_hidden_dentry);
        kfree(symbuf);
 
+       if (err)
+               goto out;
+       if (!S_ISDIR(dentry->d_inode->i_mode)) {
+               unionfs_purge_extras(dentry);
+               if (!unionfs_lower_inode(dentry->d_inode)) {
+                       /*
+                        * If we got here, then we copied up to an
+                        * unlinked-open file, whose name is .unionfsXXXXX.
+                        */
+                       struct inode *inode = new_hidden_dentry->d_inode;
+                       atomic_inc(&inode->i_count);
+                       unionfs_set_lower_inode_idx(dentry->d_inode,
+                                                   ibstart(dentry->d_inode),
+                                                   inode);
+               }
+       }
+       unionfs_inherit_mnt(dentry);
+       unionfs_check_inode(dir);
+       unionfs_check_dentry(dentry);
 out:
        return err;
 }

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index d1ee7921112f9b0c741fab5bb35a3c6bf715e2c7..1653267c1d5a5fa94861e633237f1954bcfecae8 100644 (file)
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -289,9 +289,11 @@ static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
        int err;
 
+       unionfs_check_dentry(dentry);
        unionfs_lock_dentry(dentry);
        err = __unionfs_d_revalidate_chain(dentry, nd);
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
 
        return err;
 }
@@ -304,6 +306,7 @@ static void unionfs_d_release(struct dentry *dentry)
 {
        int bindex, bstart, bend;
 
+       unionfs_check_dentry(dentry);
        /* this could be a negative dentry, so check first */
        if (!UNIONFS_D(dentry)) {
                printk(KERN_DEBUG "unionfs: dentry without private data: %.*s",

diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 2e5ec427e542b61a3f241381c4802f2cd8af6c8b..7c0553c951f8c601058ccc7c98341014859f6e82 100644 (file)
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -50,6 +50,7 @@ static loff_t unionfs_llseek(struct file *file, loff_t offset, int origin)
        }
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -74,6 +75,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
 
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -126,6 +128,7 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf,
                inode->i_size = pos;
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -156,6 +159,7 @@ static unsigned int unionfs_poll(struct file *file, poll_table *wait)
 
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return mask;
 }
 
@@ -193,6 +197,7 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -219,6 +224,7 @@ static int unionfs_fsync(struct file *file, struct dentry *dentry,
 
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 
@@ -238,6 +244,7 @@ static int unionfs_fasync(int fd, struct file *file, int flag)
 
 out:
        unionfs_read_unlock(file->f_dentry->d_sb);
+       unionfs_check_file(file);
        return err;
 }
 

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 872a6e636c98f6bbb0848ac5fdab481970fbbe0f..02bea8f31a508887302c7e04a94f281e9a5be982 100644 (file)
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -222,8 +222,14 @@ out:
        dput(wh_dentry);
        kfree(name);
 
+       if (!err)
+               unionfs_inherit_mnt(dentry);
        unionfs_unlock_dentry(dentry);
        unionfs_read_unlock(dentry->d_sb);
+
+       unionfs_check_inode(parent);
+       unionfs_check_dentry(dentry->d_parent);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -248,7 +254,11 @@ static struct dentry *unionfs_lookup(struct inode *parent,
                nd->dentry = path_save.dentry;
                nd->mnt = path_save.mnt;
        }
+       if (!IS_ERR(ret))
+               unionfs_inherit_mnt(dentry);
 
+       unionfs_check_inode(parent);
+       unionfs_check_dentry(dentry);
        return ret;
 }
 
@@ -374,10 +384,15 @@ out:
                d_drop(new_dentry);
 
        kfree(name);
+       if (!err)
+               unionfs_inherit_mnt(new_dentry);
 
        unionfs_unlock_dentry(new_dentry);
        unionfs_unlock_dentry(old_dentry);
 
+       unionfs_check_inode(dir);
+       unionfs_check_dentry(new_dentry);
+       unionfs_check_dentry(old_dentry);
        return err;
 }
 
@@ -520,7 +535,12 @@ out:
                d_drop(dentry);
 
        kfree(name);
+       if (!err)
+               unionfs_inherit_mnt(dentry);
        unionfs_unlock_dentry(dentry);
+
+       unionfs_check_inode(dir);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -654,6 +674,8 @@ out:
        kfree(name);
 
        unionfs_unlock_dentry(dentry);
+       unionfs_check_inode(parent);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -763,7 +785,12 @@ out:
 
        kfree(name);
 
+       if (!err)
+               unionfs_inherit_mnt(dentry);
        unionfs_unlock_dentry(dentry);
+
+       unionfs_check_inode(dir);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -792,6 +819,7 @@ static int unionfs_readlink(struct dentry *dentry, char __user *buf,
 
 out:
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -826,12 +854,14 @@ static void *unionfs_follow_link(struct dentry *dentry, struct nameidata *nd)
        err = 0;
 
 out:
+       unionfs_check_dentry(dentry);
        return ERR_PTR(err);
 }
 
 static void unionfs_put_link(struct dentry *dentry, struct nameidata *nd,
                             void *cookie)
 {
+       unionfs_check_dentry(dentry);
        kfree(nd_get_link(nd));
 }
 
@@ -955,6 +985,7 @@ static int unionfs_permission(struct inode *inode, int mask,
 
 out:
        unionfs_read_unlock(inode->i_sb);
+       unionfs_check_inode(inode);
        return err;
 }
 
@@ -1021,9 +1052,9 @@ static int unionfs_setattr(struct dentry *dentry, struct iattr *ia)
        hidden_inode = unionfs_lower_inode(dentry->d_inode);
        fsstack_copy_attr_all(inode, hidden_inode, unionfs_get_nlinks);
        fsstack_copy_inode_size(inode, hidden_inode);
-
 out:
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }
 

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index edc5a5cf9b9d0bd6c7bb338edb577428bc8e620a..a19957fa7889b9637b21ce14c32e9dd48ab9051c 100644 (file)
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -449,18 +449,46 @@ int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
                }
        }
        err = do_unionfs_rename(old_dir, old_dentry, new_dir, new_dentry);
-
 out:
        if (err)
                /* clear the new_dentry stuff created */
                d_drop(new_dentry);
-       else
+       else {
                /*
                 * force re-lookup since the dir on ro branch is not renamed,
                 * and hidden dentries still indicate the un-renamed ones.
                 */
                if (S_ISDIR(old_dentry->d_inode->i_mode))
                        atomic_dec(&UNIONFS_D(old_dentry)->generation);
+               else
+                       unionfs_purge_extras(old_dentry);
+               if (new_dentry->d_inode &&
+                   !S_ISDIR(new_dentry->d_inode->i_mode)) {
+                       unionfs_purge_extras(new_dentry);
+                       unionfs_inherit_mnt(new_dentry);
+                       if (!unionfs_lower_inode(new_dentry->d_inode)) {
+                               /*
+                                * If we get here, it means that no copyup
+                                * was needed, and that a file by the old
+                                * name already existing on the destination
+                                * branch; that file got renamed earlier in
+                                * this function, so all we need to do here
+                                * is set the lower inode.
+                                */
+                               struct inode *inode;
+                               inode = unionfs_lower_inode(old_dentry->d_inode);
+                               atomic_inc(&inode->i_count);
+                               unionfs_set_lower_inode_idx(
+                                       new_dentry->d_inode,
+                                       dbstart(new_dentry), inode);
+                       }
+
+               }
+               unionfs_check_inode(old_dir);
+               unionfs_check_inode(new_dir);
+               unionfs_check_dentry(old_dentry);
+               unionfs_check_dentry(new_dentry);
+       }
 
        unionfs_unlock_dentry(new_dentry);
        unionfs_unlock_dentry(old_dentry);

diff --git a/fs/unionfs/unlink.c b/fs/unionfs/unlink.c
index 2052270be53a954e8975f682571f9352eac20f8f..b3d814d384e74247455f3c0b24798443cdc93234 100644 (file)
--- a/fs/unionfs/unlink.c
+++ b/fs/unionfs/unlink.c
@@ -75,12 +75,17 @@ int unionfs_unlink(struct inode *dir, struct dentry *dentry)
 
        BUG_ON(!is_valid_dentry(dentry));
 
+       unionfs_check_dentry(dentry);
        unionfs_lock_dentry(dentry);
 
        err = unionfs_unlink_whiteout(dir, dentry);
        /* call d_drop so the system "forgets" about us */
-       if (!err)
+       if (!err) {
+               if (!S_ISDIR(dentry->d_inode->i_mode))
+                       unionfs_purge_extras(dentry);
+               unionfs_check_dentry(dentry);
                d_drop(dentry);
+       }
 
        unionfs_unlock_dentry(dentry);
        return err;
@@ -125,6 +130,7 @@ int unionfs_rmdir(struct inode *dir, struct dentry *dentry)
 
        BUG_ON(!is_valid_dentry(dentry));
 
+       unionfs_check_dentry(dentry);
        unionfs_lock_dentry(dentry);
 
        /* check if this unionfs directory is empty or not */

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
index 12d618bd8f498dc2e632a3891edad2f6cb833768..4cacead60c61a7d7053832068b231b5b386fdb9a 100644 (file)
--- a/fs/unionfs/xattr.c
+++ b/fs/unionfs/xattr.c
@@ -66,6 +66,7 @@ ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
        err = vfs_getxattr(hidden_dentry, (char*) name, value, size);
 
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -88,6 +89,7 @@ int unionfs_setxattr(struct dentry *dentry, const char *name,
                           size, flags);
 
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -108,6 +110,7 @@ int unionfs_removexattr(struct dentry *dentry, const char *name)
        err = vfs_removexattr(hidden_dentry, (char*) name);
 
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }
 
@@ -131,5 +134,6 @@ ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
        err = vfs_listxattr(hidden_dentry, encoded_list, size);
 
        unionfs_unlock_dentry(dentry);
+       unionfs_check_dentry(dentry);
        return err;
 }