Commit bd4928ec authored by Trond Myklebust's avatar Trond Myklebust
Browse files

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: default avatarAiden Lambert <alambert48@gatech.edu>
Fixes: 59a707b0 ("NFS: Ensure we revalidate the inode correctly after remove or rename")
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent 6a23ae0a
Loading
Loading
Loading
Loading
+13 −6
Original line number Diff line number Diff line
@@ -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 @@ int nfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
			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