NFS: Avoid changing nlink when file removes and attribute updates race

If a file removal races with another operation that updates its
attributes, then skip the change to nlink, and just mark the attributes
as being stale.

Reported-by: Aiden Lambert <alambert48@gatech.edu>
Fixes: 59a707b0d42e ("NFS: Ensure we revalidate the inode correctly after remove or rename")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ea9f6ca8f30fa250425921b403d67d05fcf13b61..d557b0443e8b0411811f75b96d3f015b2b2339d5 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1894,13 +1894,15 @@ static int nfs_dentry_delete(const struct dentry *dentry)
 }
 
 /* Ensure that we revalidate inode->i_nlink */
-static void nfs_drop_nlink(struct inode *inode)
+static void nfs_drop_nlink(struct inode *inode, unsigned long gencount)
 {
+       struct nfs_inode *nfsi = NFS_I(inode);
+
        spin_lock(&inode->i_lock);
        /* drop the inode if we're reasonably sure this is the last link */
-       if (inode->i_nlink > 0)
+       if (inode->i_nlink > 0 && gencount == nfsi->attr_gencount)
                drop_nlink(inode);
-       NFS_I(inode)->attr_gencount = nfs_inc_attr_generation_counter();
+       nfsi->attr_gencount = nfs_inc_attr_generation_counter();
        nfs_set_cache_invalid(
                inode, NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
                               NFS_INO_INVALID_NLINK);
@@ -1914,8 +1916,9 @@ static void nfs_drop_nlink(struct inode *inode)
 static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
 {
        if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
+               unsigned long gencount = READ_ONCE(NFS_I(inode)->attr_gencount);
                nfs_complete_unlink(dentry, inode);
-               nfs_drop_nlink(inode);
+               nfs_drop_nlink(inode, gencount);
        }
        iput(inode);
 }
@@ -2507,9 +2510,11 @@ static int nfs_safe_remove(struct dentry *dentry)
 
        trace_nfs_remove_enter(dir, dentry);
        if (inode != NULL) {
+               unsigned long gencount = READ_ONCE(NFS_I(inode)->attr_gencount);
+
                error = NFS_PROTO(dir)->remove(dir, dentry);
                if (error == 0)
-                       nfs_drop_nlink(inode);
+                       nfs_drop_nlink(inode, gencount);
        } else
                error = NFS_PROTO(dir)->remove(dir, dentry);
        if (error == -ENOENT)
@@ -2709,6 +2714,7 @@ int nfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 {
        struct inode *old_inode = d_inode(old_dentry);
        struct inode *new_inode = d_inode(new_dentry);
+       unsigned long new_gencount = 0;
        struct dentry *dentry = NULL;
        struct rpc_task *task;
        bool must_unblock = false;
@@ -2761,6 +2767,7 @@ int nfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
                } else {
                        block_revalidate(new_dentry);
                        must_unblock = true;
+                       new_gencount = NFS_I(new_inode)->attr_gencount;
                        spin_unlock(&new_dentry->d_lock);
                }
 
@@ -2800,7 +2807,7 @@ out:
                        new_dir, new_dentry, error);
        if (!error) {
                if (new_inode != NULL)
-                       nfs_drop_nlink(new_inode);
+                       nfs_drop_nlink(new_inode, new_gencount);
                /*
                 * The d_move() should be here instead of in an async RPC completion
                 * handler because we need the proper locks to move the dentry.  If