> On Jan 18, 2023, at 12:54 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Thu, 2023-01-05 at 07:15 -0500, Jeff Layton wrote:
>> 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@xxxxxxxxxx>
>> ---
>> 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
