Unionfs: reduce number of whiteouts by deleting all instances of files
authorErez Zadok <ezk@cs.sunysb.edu>
Sun, 23 Mar 2008 02:49:29 +0000 (22:49 -0400)
committerErez Zadok <ezk@cs.sunysb.edu>
Fri, 12 Aug 2011 02:37:53 +0000 (22:37 -0400)
Optimize the unlinking of non-dir objects in unionfs by deleting all
possible lower inode objects from all writable lower branches.  This may
consume a bit more processing, but on average reduces overall inode
consumption and further saves a lot by reducing the total number of
whiteouts needed.  We create a whiteout now only if we could not delete all
lower objects, or if one of the lower branches was explicitly marked
read-only.

Signed-off-by: Himanshu Kanda <hkanda@cs.sunysb.edu>
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Documentation/filesystems/unionfs/concepts.txt
fs/unionfs/lookup.c
fs/unionfs/unlink.c

index 8d9a1c5c475f08376ef79d7d560496ad936d4c1d..0bc1a19ee657849d9f878865e97d58165db2be11 100644 (file)
@@ -62,6 +62,31 @@ simplest solution is to take the instance from the highest priority
 (numerically lowest value) and "hide" the others.
 
 
+Unlinking:
+=========
+
+Unlink operation on non-directory instances is optimized to remove the
+maximum possible objects in case multiple underlying branches have the same
+file name.  The unlink operation will first try to delete file instances
+from highest priority branch and then move further to delete from remaining
+branches in order of their decreasing priority.  Consider a case (F..D..F),
+where F is a file and D is a directory of the same name; here, some
+intermediate branch could have an empty directory instance with the same
+name, so this operation also tries to delete this directory instance and
+proceed further to delete from next possible lower priority branch.  The
+unionfs unlink operation will smoothly delete the files with same name from
+all possible underlying branches.  In case if some error occurs, it creates
+whiteout in highest priority branch that will hide file instance in rest of
+the branches.  An error could occur either if an unlink operations in any of
+the underlying branch failed or if a branch has no write permission.
+
+This unlinking policy is known as "delete all" and it has the benefit of
+overall reducing the number of inodes used by duplicate files, and further
+reducing the total number of inodes consumed by whiteouts.  The cost is of
+extra processing, but testing shows this extra processing is well worth the
+savings.
+
+
 Copyup:
 =======
 
index 755158e9ce28717cd0b37da5a457cd1e807d7660..7618716cf0ad04ef3e7e22dc98d8b143c08e02e4 100644 (file)
@@ -264,7 +264,9 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
                 * branches, but we have to skip non-dirs (to avoid, say,
                 * calling readdir on a regular file).
                 */
-               if (!S_ISDIR(lower_dentry->d_inode->i_mode) && dentry_count) {
+               if ((lookupmode != INTERPOSE_PARTIAL) &&
+                   !S_ISDIR(lower_dentry->d_inode->i_mode) &&
+                   dentry_count) {
                        dput(lower_dentry);
                        continue;
                }
@@ -295,10 +297,6 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry,
                                continue;
                        if (dentry_count == 1)
                                goto out_positive;
-                       /* This can only happen with mixed D-*-F-* */
-                       BUG_ON(!S_ISDIR(unionfs_lower_dentry(dentry)->
-                                       d_inode->i_mode));
-                       continue;
                }
 
                opaque = is_opaque_dir(dentry, bindex);
index 6e93da320e6851fa7fb249556690a406ecc56030..c66bb3ee2d291ec322e02ed01b6adf7da4d6c174 100644 (file)
 
 #include "union.h"
 
-/* unlink a file by creating a whiteout */
+/*
+ * Helper function for Unionfs's unlink operation.
+ *
+ * The main goal of this function is to optimize the unlinking of non-dir
+ * objects in unionfs by deleting all possible lower inode objects from the
+ * underlying branches having same dentry name as the non-dir dentry on
+ * which this unlink operation is called.  This way we delete as many lower
+ * inodes as possible, and save space.  Whiteouts need to be created in
+ * branch0 only if unlinking fails on any of the lower branch other than
+ * branch0, or if a lower branch is marked read-only.
+ *
+ * Also, while unlinking a file, if we encounter any dir type entry in any
+ * intermediate branch, then we remove the directory by calling vfs_rmdir.
+ * The following special cases are also handled:
+
+ * (1) If an error occurs in branch0 during vfs_unlink, then we return
+ *     appropriate error.
+ *
+ * (2) If we get an error during unlink in any of other lower branch other
+ *     than branch0, then we create a whiteout in branch0.
+ *
+ * (3) If a whiteout already exists in any intermediate branch, we delete
+ *     all possible inodes only up to that branch (this is an "opaqueness"
+ *     as as per Documentation/filesystems/unionfs/concepts.txt).
+ *
+ */
 static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
 {
        struct dentry *lower_dentry;
@@ -30,51 +55,57 @@ static int unionfs_unlink_whiteout(struct inode *dir, struct dentry *dentry)
        if (err)
                goto out;
 
-       bindex = dbstart(dentry);
-
-       lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
-       if (!lower_dentry)
-               goto out;
+       /* trying to unlink all possible valid instances */
+       for (bindex = dbstart(dentry); bindex <= dbend(dentry); bindex++) {
+               lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+               if (!lower_dentry || !lower_dentry->d_inode)
+                       continue;
+
+               lower_dir_dentry = lock_parent(lower_dentry);
+
+               /* avoid destroying the lower inode if the object is in use */
+               dget(lower_dentry);
+               err = is_robranch_super(dentry->d_sb, bindex);
+               if (!err) {
+                       /* see Documentation/filesystems/unionfs/issues.txt */
+                       lockdep_off();
+                       if (!S_ISDIR(lower_dentry->d_inode->i_mode))
+                               err = vfs_unlink(lower_dir_dentry->d_inode,
+                                                               lower_dentry);
+                       else
+                               err = vfs_rmdir(lower_dir_dentry->d_inode,
+                                                               lower_dentry);
+                       lockdep_on();
+               }
 
-       lower_dir_dentry = lock_parent(lower_dentry);
+               /* if lower object deletion succeeds, update inode's times */
+               if (!err)
+                       unionfs_copy_attr_times(dentry->d_inode);
+               dput(lower_dentry);
+               fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
+               unlock_dir(lower_dir_dentry);
 
-       /* avoid destroying the lower inode if the file is in use */
-       dget(lower_dentry);
-       err = is_robranch_super(dentry->d_sb, bindex);
-       if (!err) {
-               /* see Documentation/filesystems/unionfs/issues.txt */
-               lockdep_off();
-               err = vfs_unlink(lower_dir_dentry->d_inode, lower_dentry);
-               lockdep_on();
+               if (err)
+                       break;
        }
-       /* if vfs_unlink succeeded, update our inode's times */
-       if (!err)
-               unionfs_copy_attr_times(dentry->d_inode);
-       dput(lower_dentry);
-       fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-       unlock_dir(lower_dir_dentry);
-
-       if (err && !IS_COPYUP_ERR(err))
-               goto out;
 
        /*
-        * We create whiteouts if (1) there was an error unlinking the main
-        * file; (2) there is a lower priority file with the same name
-        * (dbopaque); (3) the branch in which the file is not the last
-        * (rightmost0 branch.  The last rule is an optimization to avoid
-        * creating all those whiteouts if there's no chance they'd be
-        * masking any lower-priority branch, as well as unionfs is used
-        * with only one branch (using only one branch, while odd, is still
-        * possible).
+        * Create the whiteout in branch 0 (highest priority) only if (a)
+        * there was an error in any intermediate branch other than branch 0
+        * due to failure of vfs_unlink/vfs_rmdir or (b) a branch marked or
+        * mounted read-only.
         */
        if (err) {
-               if (dbstart(dentry) == 0)
+               if ((bindex == 0) ||
+                   ((bindex == dbstart(dentry)) &&
+                    (!IS_COPYUP_ERR(err))))
                        goto out;
-               err = create_whiteout(dentry, dbstart(dentry) - 1);
-       } else if (dbopaque(dentry) != -1) {
-               err = create_whiteout(dentry, dbopaque(dentry));
-       } else if (dbstart(dentry) < sbend(dentry->d_sb)) {
-               err = create_whiteout(dentry, dbstart(dentry));
+               else {
+                       if (!IS_COPYUP_ERR(err))
+                               pr_debug("unionfs: lower object deletion "
+                                            "failed in branch:%d\n", bindex);
+                       err = create_whiteout(dentry, sbstart(dentry->d_sb));
+               }
        }
 
 out: