Commit 0c8c7c55 authored by Scott Mayhew's avatar Scott Mayhew Committed by Trond Myklebust
Browse files

nfs: don't invalidate dentries on transient errors



This is a slight variation on a patch previously proposed by Neil Brown
that never got merged.

Prior to commit 5ceb9d7f ("NFS: Refactor nfs_lookup_revalidate()"),
any error from nfs_lookup_verify_inode() other than -ESTALE would result
in nfs_lookup_revalidate() returning that error (-ESTALE is mapped to
zero).

Since that commit, all errors result in nfs_lookup_revalidate()
returning zero, resulting in dentries being invalidated where they
previously were not (particularly in the case of -ERESTARTSYS).

Fix it by passing the actual error code to nfs_lookup_revalidate_done(),
and leaving the decision on whether to  map the error code to zero or
one to nfs_lookup_revalidate_done().

A simple reproducer is to run the following python code in a
subdirectory of an NFS mount (not in the root of the NFS mount):

---8<---
import os
import multiprocessing
import time

if __name__=="__main__":
    multiprocessing.set_start_method("spawn")

    count = 0
    while True:
        try:
            os.getcwd()
            pool = multiprocessing.Pool(10)
            pool.close()
            pool.terminate()
            count += 1
        except Exception as e:
            print(f"Failed after {count} iterations")
            print(e)
            break
---8<---

Prior to commit 5ceb9d7f, the above code would run indefinitely.
After commit 5ceb9d7f, it fails almost immediately with -ENOENT.

Signed-off-by: default avatarScott Mayhew <smayhew@redhat.com>
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@hammerspace.com>
parent a527c3ba
Loading
Loading
Loading
Loading
+13 −14
Original line number Diff line number Diff line
@@ -1627,7 +1627,16 @@ nfs_lookup_revalidate_done(struct inode *dir, struct dentry *dentry,
	switch (error) {
	case 1:
		break;
	case 0:
	case -ETIMEDOUT:
		if (inode && (IS_ROOT(dentry) ||
			      NFS_SERVER(inode)->flags & NFS_MOUNT_SOFTREVAL))
			error = 1;
		break;
	case -ESTALE:
	case -ENOENT:
		error = 0;
		fallthrough;
	default:
		/*
		 * We can't d_drop the root of a disconnected tree:
		 * its d_hash is on the s_anon list and d_drop() would hide
@@ -1682,18 +1691,8 @@ static int nfs_lookup_revalidate_dentry(struct inode *dir,

	dir_verifier = nfs_save_change_attribute(dir);
	ret = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr);
	if (ret < 0) {
		switch (ret) {
		case -ESTALE:
		case -ENOENT:
			ret = 0;
			break;
		case -ETIMEDOUT:
			if (NFS_SERVER(inode)->flags & NFS_MOUNT_SOFTREVAL)
				ret = 1;
		}
	if (ret < 0)
		goto out;
	}

	/* Request help from readdirplus */
	nfs_lookup_advise_force_readdirplus(dir, flags);
@@ -1737,7 +1736,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
			 unsigned int flags)
{
	struct inode *inode;
	int error;
	int error = 0;

	nfs_inc_stats(dir, NFSIOS_DENTRYREVALIDATE);
	inode = d_inode(dentry);
@@ -1782,7 +1781,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
out_bad:
	if (flags & LOOKUP_RCU)
		return -ECHILD;
	return nfs_lookup_revalidate_done(dir, dentry, inode, 0);
	return nfs_lookup_revalidate_done(dir, dentry, inode, error);
}

static int