On Fri, Dec 19, 2025 at 12:25 PM Jeff Layton <[email protected]> wrote: > > On Fri, 2025-12-19 at 10:58 -0500, Olga Kornievskaia wrote: > > Hi Jeff, > > > > I narrowed down the upstream failure for generic/215 and generic/407 > > to this commit. > > > > Let's consider first where the kernel is compiled with delegated > > attributes off (but it also fails just the same if the delegated > > attributes are compiled in). > > > > I don't understand why the code unconditionally changed to call > > nfsd4_finalize_deleg_timestamps() which I think the main driver behind > > the failure. > > > > Running generic/407 there is an OPEN (which gives out a write > > delegation) and returns a change id, then on this filehandle there is > > a SETATTR (with a getattr) which returns a new changeid. Then there is > > a CLONE where the filehandle is the destination filehandle on which > > there is a getattr which returns unchanged changeid/modify time (bad). > > Then there is a DELEGRETURN (with a getattr) which again returns same > > change id. Test fails. > > > > Prior to this commit. The changeid/modify time is different in CLONE > > and DELEGRETURN -- test passes. > > > > Now let me describe what happens with delegated attributes enabled. > > OPEN returns delegated attributes delegation, included getattr return > > a changeid. Then CLONE is done, the included gettattr returns a > > different (from open's) changeid (different time_modify). Then there > > is SETATTR+GEATTR+DELEGRETURN compound from the client (which carries > > a time_deleg_modify value different from above). Server in getattr > > replies with changeid same as in clone and mtime with the value client > > provided. So I'm not sure exactly why the test fails here but that's a > > different problem as my focus is on "delegation attribute off option" > > at the moment. > > > > I don't know if this is the correct fix or not but perhaps we > > shouldn't unconditionally be setting this mode? (note this fix only > > fixes the delegattributes off. however i have no claims that this > > patch is what broke 215/407 for delegated attributes on. Something > > else is in play there). If this solution is acceptable, I can send a > > patch. > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index 81fa7cc6c77b..624cc6ab2802 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -6318,7 +6318,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp, > > struct nfsd4_open *open, > > dp->dl_ctime = stat.ctime; > > dp->dl_mtime = stat.mtime; > > spin_lock(&f->f_lock); > > - f->f_mode |= FMODE_NOCMTIME; > > + if (deleg_ts) > > + f->f_mode |= FMODE_NOCMTIME; > > spin_unlock(&f->f_lock); > > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > > } else { > > > > > > That patch does look correct to me -- nice catch. Have you validated > that it fixes 215 and 407?
Yes, it does fix 215 and 407 for me. > > Thanks, > Jeff > -- > Jeff Layton <[email protected]>
