Skip to content

Commit

Permalink
nfs: don't invalidate dentries on transient errors
Browse files Browse the repository at this point in the history
This is a slight variation on a patch previously proposed by Neil Brown
that never got merged.

Prior to commit 5ceb9d7 ("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 5ceb9d7, the above code would run indefinitely.
After commit 5ceb9d7, it fails almost immediately with -ENOENT.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
  • Loading branch information
Scott Mayhew authored and Trond Myklebust committed May 24, 2024
1 parent a527c3b commit 0c8c7c5
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions fs/nfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0c8c7c5

Please sign in to comment.