On Fri, 2024-09-20 at 18:18 +0000, Chuck Lever III wrote:
>
> > On Sep 20, 2024, at 12:34 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >
> > On Fri, 2024-09-20 at 16:30 +0000, Chuck Lever III wrote:
> > >
> > > > On Sep 20, 2024, at 12:24 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > > >
> > > > On Fri, 2024-09-20 at 12:05 -0400, Olga Kornievskaia wrote:
> > > > > Current, the server returns that it supports NFS4_SHARE_WANT_DELEG_TIMESTAMPS
> > > > > but when the client sends that on the open, knfsd returns back with
> > > > > bad_xdr error.
> > > > >
> > > > > Fixes: d0eab73d48a0 ("nfsd: add support for delegated timestamps")
> > > > > Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> > > > > ---
> > > > > fs/nfsd/nfs4xdr.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > > > index c0bad580ab6d..adda8b489175 100644
> > > > > --- a/fs/nfsd/nfs4xdr.c
> > > > > +++ b/fs/nfsd/nfs4xdr.c
> > > > > @@ -1109,6 +1109,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
> > > > > case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
> > > > > case (NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL |
> > > > > NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED):
> > > > > + case NFS4_SHARE_WANT_DELEG_TIMESTAMPS:
> > > > > return nfs_ok;
> > > > > }
> > > > > return nfserr_bad_xdr;
> > > >
> > > > Ouch.
> > > >
> > > > The problem here is that we had to drop the patch that added
> > > > OPEN_XOR_DELEGATION support. That patch reworked the flag handling in
> > > > this function in a way that allowed the new delstid flags to be
> > > > properly supported.
> > > >
> > > > I think we probably want to resurrect the parts of this patch that
> > > > alter nfsd4_decode_share_access:
> > > >
> > > > https://lore.kernel.org/linux-nfs/20240905-delstid-v4-8-d3e5fd34d107@xxxxxxxxxx/
> > > >
> > > > Olga, would you be OK with that approach instead?
> > >
> > > At this point, I'd like to consider postponing the delstid patches
> > > until v6.13. They haven't gotten enough testing in their current
> > > form...
> >
> > It's your call, but that seems like an extreme reaction to a flag
> > handling fix, given that we have several weeks of -rc's ahead of us.
>
> It's not just this small problem that worries me.
>
> I pulled in v4 of the delstid patches pretty much at the last
> moment for v6.12 development. So far, there's been an 80%
> performance regression, and the surgery to work around that
> issue introduced another bug that invalidates all NFSv4.2 test
> results since that surgery, just a week ago.
>
> The delstid patches impact an already heavily-used code path
> (OPEN). You've already found one or two bugs in the client
> side as well. This isn't something the can be disabled by
> distros until it is ready -- it's baked into a popular code
> path as soon as it goes upstream, and NFSv4.2 is the current
> default.
>
> So I can't say with confidence that this code is stable and
> ready to merge. If we had another week or two before v6.11
> final, there wouldn't be a question, but it's already a week
> into the v6.12 merge window, and I need to submit a pull
> request in the next few days. A 6.11-rc8 would have been
> extremely helpful, but we didn't get one.
>
> Exposing this change set to the cauldron that is bake-a-thon
> would be valuable, IMO, for both the client and the server.
>
Fair enough. These are not features that anyone is particularly
clamoring for, so dropping them for now is fine. I'll plan to spin up a
new set once I'm back from travel (another week or so).
--
Jeff Layton <jlayton@xxxxxxxxxx>
