Wrapfs: better handling of NFS silly-renamed files

In ->unlink, if we try to unlink an NFS silly-renamed file, NFS returns
-EBUSY.  We have to treat it as a success and return 0 to the VFS.  NFS will
remove silly-deleted files later on anyway.

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

diff --git a/Documentation/filesystems/wrapfs.txt b/Documentation/filesystems/wrapfs.txt
index 43b4e613083559ec81ce64f83e43dcd1edf4b0bf..f61879a20a266bc30f2bd79433fc47da310dc40f 100644 (file)
--- a/Documentation/filesystems/wrapfs.txt
+++ b/Documentation/filesystems/wrapfs.txt
@@ -106,15 +106,13 @@ CAVEATS:
 Stacking on NFS.  Wrapfs has been tested with LTP, racer, fsx, parallel
 compile, and more.  It's been tested on top of ext2, ext3, xfs, reiserfs,
 and tmpfs -- and passed all tests.  However, on top of nfs3, wrapfs has to
-treat silly-deleted files as if they don't exist; the VFS also has special
-handling for silly-deleted files, so this isn't unusual.  Even so, there are
-sometimes races between readdir and lower files which may have gotten
-unlinked: an "rm -fr" may return ENOENT for entries which show up in
-readdir, but were already unlinked on NFS below.  It's mainly harmless.
-This could be solved easily if the VFS were to pass an "unlink/rmdir" namei
-intent to the file system: then wrapfs could detect that the intent was to
-remove the lower object, and ignore ENOENT errors from the lower file
-system.
+treat silly-deleted files as if they don't exist: in ->unlink, if we try to
+vfs_unlink an NFS silly-deleted file, NFS returns EBUSY; so we simply ignore
+it and return 0 (success) to the VFS.  NFS will delete this file later on
+anyway.  As the VFS also has special handling for silly-deleted files, this
+isn't unusual.  A cleaner way to handle this in the future is if the VFS
+were to handle silly-deleted (aka "delayed-delete") files entirely at the
+VFS.
 
 ------------------------------------------------------------------------------
 HISTORY:

diff --git a/fs/wrapfs/inode.c b/fs/wrapfs/inode.c
index 0eb895b874b49f385a1ac14d788ce82453f296c2..8374d9c440a92ab8490ba6c5b89e676a3c1741cf 100644 (file)
--- a/fs/wrapfs/inode.c
+++ b/fs/wrapfs/inode.c
@@ -107,22 +107,20 @@ static int wrapfs_unlink(struct inode *dir, struct dentry *dentry)
        dget(lower_dentry);
        lower_dir_dentry = lock_parent(lower_dentry);
 
-       /*
-        * If the lower dentry is already silly-renamed (NFS), then we treat
-        * this unlink operation as if it already succeeded, and don't
-        * actually call vfs_unlink on the lower dentry (NFS may return
-        * EBUSY).  Note: VFS also has special handling for silly-renamed
-        * files, so this treatment isn't unusual.
-        */
-       if (lower_dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-               err = 0;
-               goto out_unlock;
-       }
        err = mnt_want_write(lower_path.mnt);
        if (err)
                goto out_unlock;
        err = vfs_unlink(lower_dir_inode, lower_dentry);
-       /* Note: unlinking on top of NFS can cause silly-renamed files */
+
+       /*
+        * Note: unlinking on top of NFS can cause silly-renamed files.
+        * Trying to delete such files results in EBUSY from NFS
+        * below.  Silly-renamed files will get deleted by NFS later on, so
+        * we just need to detect them here and treat such EBUSY errors as
+        * if the upper file was successfully deleted.
+        */
+       if (err == -EBUSY && lower_dentry->d_flags & DCACHE_NFSFS_RENAMED)
+               err = 0;
        if (err)
                goto out;
        fsstack_copy_attr_times(dir, lower_dir_inode);
@@ -218,11 +216,6 @@ static int wrapfs_rmdir(struct inode *dir, struct dentry *dentry)
        lower_dentry = lower_path.dentry;
        lower_dir_dentry = lock_parent(lower_dentry);
 
-       /* see comment about silly-renamed files in wrapfs_unlink */
-       if (lower_dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-               err = 0;
-               goto out_unlock;
-       }
        err = mnt_want_write(lower_path.mnt);
        if (err)
                goto out_unlock;