On Fri, Dec 19, 2025 at 12:25 PM Jeff Layton [off-list ref] wrote:

On Fri, 2025-12-19 at 10:58 -0500, Olga Kornievskaia wrote:

quoted

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 [off-list ref]