On 8/11/25 3:56 PM, Jeff Layton wrote:
> On Mon, 2025-08-11 at 15:32 -0400, Olga Kornievskaia wrote:
>> On Mon, Aug 11, 2025 at 3:10 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>>
>>> On Mon, 2025-08-11 at 14:18 -0400, 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.
>>>>
>>>> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
>>>> ---
>>>> fs/nfsd/lockd.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
>>>> index edc9f75dc75c..ad3e461f30c0 100644
>>>> --- a/fs/nfsd/lockd.c
>>>> +++ b/fs/nfsd/lockd.c
>>>> @@ -57,6 +57,7 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
>>>> switch (nfserr) {
>>>> case nfs_ok:
>>>> return 0;
>>>> + case nfserr_jukebox:
>>>> case nfserr_dropit:
>>>> return nlm_drop_reply;
>>>> case nfserr_stale:
>>>
>>> This works by triggering a RPC retransmission. That could time out on
>>> soft mounts if it takes a while to return a delegation. Looking at the
>>> NLM spec here:
>>>
>>> https://pubs.opengroup.org/onlinepubs/9629799/chap14.htm
>>>
>>> What about returning NLM4_DENIED instead? The description there is:
>>>
>>> NLM4_DENIED
>>> The call failed. For attempts to set a lock, this status implies
>>> that if the client retries the call later, it may succeed.
>>>
>>> Presumably the client should redrive this effectively indefinitely that
>>> way?
>>
>> I have previously tried "denied" but that lead to client failing with
>> no lock just like from nlm_failed instead of retrying. I think only
>> blocked (or block_grace) leads to a retry.
>>
>
> Thanks, I noticed after I sent the above.
>
> I think that the ideal thing to do would be to treat this like a
> blocked lock and try to call the client back when the delegation is
> returned. That may be difficult to do properly though, since you won't
> have a proper lock request to block on.
>
> For now, this is a reasonable solution, but it might be good to have a
> comment that explains why the other options were unacceptable.
+1 -- either a comment in the code, or please add the explanation to the
patch description.
> You can add this to both patches:
>
> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
--
Chuck Lever
