On Thu, Aug 21, 2025 at 3:45 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
> On 8/21/25 3:28 PM, Olga Kornievskaia wrote:
> > On Thu, Aug 21, 2025 at 3:18 PM Chuck Lever <chuck.lever@xxxxxxxxxx> 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")
That sounds fine to me.
> Suggested-Cc: stable@xxxxxxxxxxxxxxx # v6.6
>
> (I'll remove the "Suggested-" when applying the patch)
I will re-send original patch with the Fixes and Suggested-Cc added to
the commit message.
> >>> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
> >>> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> >>> ---
> >>> 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 believe this fix is appropriate for LTS.
> 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
