On Jan 18, 2023, at 12:54 PM, Jeff Layton [off-list ref] wrote:

On Thu, 2023-01-05 at 07:15 -0500, Jeff Layton wrote:

quoted

An error from break_lease is non-fatal, so we needn't destroy the
nfsd_file in that case. Just put the reference like we normally would
and return the error.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/filecache.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index a67b22579c6e..f0ca9501edb2 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c

@@ -1113,7 +1113,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,

	nf = nfsd_file_alloc(&key, may_flags);
	if (!nf) {
		status = nfserr_jukebox;
-		goto out_status;
+		goto out;
	}

	ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,

@@ -1122,13 +1122,11 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,

	if (likely(ret == 0))
		goto open_file;

-	nfsd_file_slab_free(&nf->nf_rcu);
-	nf = NULL;
	if (ret == -EEXIST)
		goto retry;
	trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
	status = nfserr_jukebox;
-	goto out_status;
+	goto construction_err;

wait_for_construction:
	wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);

@@ -1138,28 +1136,24 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,

		trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
		if (!open_retry) {
			status = nfserr_jukebox;
-			goto out;
+			goto construction_err;
		}
		open_retry = false;
-		if (refcount_dec_and_test(&nf->nf_ref))
-			nfsd_file_free(nf);
		goto retry;
	}
-
	this_cpu_inc(nfsd_file_cache_hits);

	status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
+	if (status != nfs_ok) {
+		nfsd_file_put(nf);
+		nf = NULL;
+	}
+
out:
	if (status == nfs_ok) {
		this_cpu_inc(nfsd_file_acquisitions);
		*pnf = nf;
-	} else {
-		if (refcount_dec_and_test(&nf->nf_ref))
-			nfsd_file_free(nf);
-		nf = NULL;
	}
-
-out_status:
	put_cred(key.cred);
	trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
	return status;

@@ -1189,6 +1183,13 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,

	if (status != nfs_ok)
		nfsd_file_unhash(nf);
	clear_and_wake_up_bit(NFSD_FILE_PENDING, &nf->nf_flags);
+	if (status == nfs_ok)
+		goto out;
+
+construction_err:
+	if (refcount_dec_and_test(&nf->nf_ref))
+		nfsd_file_free(nf);
+	nf = NULL;
	goto out;
}

Chuck, ping?

You never responded to this patch and I don't see it in your tree. Any
thoughts?

It's been in nfsd-next for a bit, but I had to drop a bunch of patches
from nfsd-next until yesterday's -rc PR was applied upstream because
they depended on one of Dai's crasher fixes.

That's been sorted, and I've restored these to nfsd-next.

--
Chuck Lever