Re: [PATCH v2 1/2] nfsd: nfserr_jukebox in nlm_fopen should lead to a retry

[Chuck Lever <chuck.lever@oracle.com>]

On 8/21/25 3:28 PM, Olga Kornievskaia wrote:
> On Thu, Aug 21, 2025 at 3:18 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>>
>> OK, let's reset this discussion.
>>
>>
>> On 8/12/25 12:03 PM, Olga Kornievskaia wrote:
>>> When v3 NLM request finds a conflicting delegation, it triggers
>>> a delegation recall and nfsd_open fails with EAGAIN. nfsd_open
>>> then translates EAGAIN into nfserr_jukebox. In nlm_fopen, instead
>>> of returning nlm_failed for when there is a conflicting delegation,
>>> drop this NLM request so that the client retries. Once delegation
>>> is recalled and if a local lock is claimed, a retry would lead to
>>> nfsd returning a nlm_lck_blocked error or a successful nlm lock.
>>
>> If this patch "... solves a problem and a fix is needed" then we need a
>> Fixes: tag so the patch is prioritized and considered for LTS.
> 
> What fixes tag is appropriate here? This is day 0 behaviour but it's
> only a problem since additions of write delegations I believe.

How about:

Fixes: d343fce148a4 ("[PATCH] knfsd: Allow lockd to drop replies as
appropriate")
Suggested-Cc: stable@vger.kernel.org # v6.6

(I'll remove the "Suggested-" when applying the patch)


>>> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
>>> ---
>>>  fs/nfsd/lockd.c | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
>>> index edc9f75dc75c..8fdc769d392e 100644
>>> --- a/fs/nfsd/lockd.c
>>> +++ b/fs/nfsd/lockd.c
>>> @@ -57,6 +57,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
>>>       switch (nfserr) {
>>>       case nfs_ok:
>>>               return 0;
>>> +     case nfserr_jukebox:
>>> +             /* this error can indicate a presence of a conflicting
>>> +              * delegation to an NLM lock request. Options are:
>>> +              * (1) For now, drop this request and make the client
>>> +              * retry. When delegation is returned, client's retry will
>>> +              * complete.
>>> +              * (2) NLM4_DENIED as per "spec" signals to the client
>>> +              * that the lock is unavaiable now but client can retry.
>>> +              * Linux client implementation does not. It treats
>>> +              * NLM4_DENIED same as NLM4_FAILED and errors the request.
>>> +              * (3) For the future, treat this as blocked lock and try
>>> +              * to callback when the delegation is returned but might
>>> +              * not have a proper lock request to block on.
>>> +              */
>>
>> s/unavaiable/unavailable
>>
>> Since 2020, kernel coding style uses the "fallthrough;" keyword for
>> switch cases with no "break;".
>>
>> Although, instead of "fallthrough;",
> 
> I'll add that.
> 
>> you could simply remove the
>> nfserr_dropit case in this patch. That would remove the conflict with
>> Neil's patch (if it were to be postponed until after this one).
> 
> I can re-send the patch with the fallthrough added on top of Neil's patch.

If you agree that this fix is appropriate for LTS, then Neil's patch
needs to be rebased on top of this one to facilitate automated LTS
backporting.

I can drop Neil's patch from nfsd-testing and have him submit a
rebased one, once this one is re-applied to nfsd-testing.


>>>       case nfserr_dropit:
>>>               return nlm_drop_reply;
>>>       case nfserr_stale:
>>
>>
>>
>> --
>> Chuck Lever
>>


-- 
Chuck Lever