Allow SETATTR to handle delegated timestamps. This patch assumes that
only the delegation holder has the ability to set the timestamps in this
way, so we allow this only if the SETATTR stateid refers to a
*_ATTRS_DELEG delegation.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
fs/nfsd/nfs4state.c | 2 +-
fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
fs/nfsd/nfsd.h | 5 ++++-
4 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
.na_iattr = &setattr->sa_iattr,
.na_seclabel = &setattr->sa_label,
};
+ bool save_no_wcc, deleg_attrs;
+ struct nfs4_stid *st = NULL;
struct inode *inode;
__be32 status = nfs_ok;
- bool save_no_wcc;
int err;
- if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
+ deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
+ FATTR4_WORD2_TIME_DELEG_MODIFY);
+
+ if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
+ int flags = WR_STATE;
+
+ if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
+ flags |= RD_STATE;
+
status = nfs4_preprocess_stateid_op(rqstp, cstate,
&cstate->current_fh, &setattr->sa_stateid,
- WR_STATE, NULL, NULL);
+ flags, NULL, &st);
if (status)
return status;
}
+
+ if (deleg_attrs) {
+ status = nfserr_bad_stateid;
+ if (st->sc_type & SC_TYPE_DELEG) {
+ struct nfs4_delegation *dp = delegstateid(st);
+
+ /* Only for *_ATTRS_DELEG flavors */
+ if (deleg_attrs_deleg(dp->dl_type))
+ status = nfs_ok;
+ }
+ }
+ if (st)
+ nfs4_put_stid(st);
+ if (status)
+ return status;
+
err = fh_want_write(&cstate->current_fh);
if (err)
return nfserrno(err);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
static inline __be32
nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
{
- if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
+ if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
return nfserr_openmode;
else
return nfs_ok;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
*umask = mask & S_IRWXUGO;
iattr->ia_valid |= ATTR_MODE;
}
+ if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+ fattr4_time_deleg_access access;
+
+ if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
+ return nfserr_bad_xdr;
+ iattr->ia_atime.tv_sec = access.seconds;
+ iattr->ia_atime.tv_nsec = access.nseconds;
+ iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
+ }
+ if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+ fattr4_time_deleg_modify modify;
+
+ if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
+ return nfserr_bad_xdr;
+ iattr->ia_mtime.tv_sec = modify.seconds;
+ iattr->ia_mtime.tv_nsec = modify.nseconds;
+ iattr->ia_ctime.tv_sec = modify.seconds;
+ iattr->ia_ctime.tv_nsec = modify.seconds;
+ iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
+ }
/* request sanity: did attrlist4 contain the expected number of words? */
if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
#endif
#define NFSD_WRITEABLE_ATTRS_WORD2 \
(FATTR4_WORD2_MODE_UMASK \
- | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
+ | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
+ | FATTR4_WORD2_TIME_DELEG_ACCESS \
+ | FATTR4_WORD2_TIME_DELEG_MODIFY \
+ )
#define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
NFSD_WRITEABLE_ATTRS_WORD0
--
2.47.1On 12/9/24 4:14 PM, Jeff Layton wrote:
> Allow SETATTR to handle delegated timestamps. This patch assumes that
> only the delegation holder has the ability to set the timestamps in this
> way, so we allow this only if the SETATTR stateid refers to a
> *_ATTRS_DELEG delegation.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
> fs/nfsd/nfs4state.c | 2 +-
> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
> fs/nfsd/nfsd.h | 5 ++++-
> 4 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> .na_iattr = &setattr->sa_iattr,
> .na_seclabel = &setattr->sa_label,
> };
> + bool save_no_wcc, deleg_attrs;
> + struct nfs4_stid *st = NULL;
> struct inode *inode;
> __be32 status = nfs_ok;
> - bool save_no_wcc;
> int err;
>
> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
> + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
> + FATTR4_WORD2_TIME_DELEG_MODIFY);
> +
> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
> + int flags = WR_STATE;
> +
> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
> + flags |= RD_STATE;
> +
> status = nfs4_preprocess_stateid_op(rqstp, cstate,
> &cstate->current_fh, &setattr->sa_stateid,
> - WR_STATE, NULL, NULL);
> + flags, NULL, &st);
> if (status)
> return status;
> }
> +
> + if (deleg_attrs) {
> + status = nfserr_bad_stateid;
> + if (st->sc_type & SC_TYPE_DELEG) {
> + struct nfs4_delegation *dp = delegstateid(st);
> +
> + /* Only for *_ATTRS_DELEG flavors */
> + if (deleg_attrs_deleg(dp->dl_type))
> + status = nfs_ok;
> + }
> + }
> + if (st)
> + nfs4_put_stid(st);
> + if (status)
> + return status;
> +
> err = fh_want_write(&cstate->current_fh);
> if (err)
> return nfserrno(err);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> static inline __be32
> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
> {
> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
> return nfserr_openmode;
> else
> return nfs_ok;
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
> *umask = mask & S_IRWXUGO;
> iattr->ia_valid |= ATTR_MODE;
> }
> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> + fattr4_time_deleg_access access;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
> + return nfserr_bad_xdr;
> + iattr->ia_atime.tv_sec = access.seconds;
> + iattr->ia_atime.tv_nsec = access.nseconds;
> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
> + }
> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> + fattr4_time_deleg_modify modify;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
> + return nfserr_bad_xdr;
> + iattr->ia_mtime.tv_sec = modify.seconds;
> + iattr->ia_mtime.tv_nsec = modify.nseconds;
> + iattr->ia_ctime.tv_sec = modify.seconds;
> + iattr->ia_ctime.tv_nsec = modify.seconds;
> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
> + }
>
> /* request sanity: did attrlist4 contain the expected number of words? */
> if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
> #endif
> #define NFSD_WRITEABLE_ATTRS_WORD2 \
> (FATTR4_WORD2_MODE_UMASK \
> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
> + )
>
> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
> NFSD_WRITEABLE_ATTRS_WORD0
>
Hi Jeff-
After this patch is applied, I see failures of the git regression suite
on NFSv4.2 mounts.
Test Summary Report
-------------------
./t3412-rebase-root.sh (Wstat: 256 (exited
1) Tests: 25 Failed: 5)
Failed tests: 6, 19, 21-22, 24
Non-zero exit status: 1
./t3400-rebase.sh (Wstat: 256 (exited
1) Tests: 38 Failed: 1)
Failed test: 31
Non-zero exit status: 1
./t3406-rebase-message.sh (Wstat: 256 (exited
1) Tests: 32 Failed: 2)
Failed tests: 15, 20
Non-zero exit status: 1
./t3428-rebase-signoff.sh (Wstat: 256 (exited
1) Tests: 7 Failed: 2)
Failed tests: 6-7
Non-zero exit status: 1
./t3418-rebase-continue.sh (Wstat: 256 (exited
1) Tests: 29 Failed: 1)
Failed test: 7
Non-zero exit status: 1
./t3415-rebase-autosquash.sh (Wstat: 256 (exited
1) Tests: 27 Failed: 2)
Failed tests: 3-4
Non-zero exit status: 1
./t3404-rebase-interactive.sh (Wstat: 256 (exited
1) Tests: 131 Failed: 15)
Failed tests: 32, 34-43, 45, 121-123
Non-zero exit status: 1
./t1013-read-tree-submodule.sh (Wstat: 256 (exited
1) Tests: 68 Failed: 1)
Failed test: 34
Non-zero exit status: 1
./t2013-checkout-submodule.sh (Wstat: 256 (exited
1) Tests: 74 Failed: 4)
Failed tests: 26-27, 30-31
Non-zero exit status: 1
./t5500-fetch-pack.sh (Wstat: 256 (exited
1) Tests: 375 Failed: 1)
Failed test: 28
Non-zero exit status: 1
./t5572-pull-submodule.sh (Wstat: 256 (exited
1) Tests: 67 Failed: 2)
Failed tests: 5, 7
Non-zero exit status: 1
Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
1037.05 cusr 6529.12 csys = 7587.52 CPU)
Result: FAIL
The NFS client and NFS server under test are running the same v6.13-rc2
kernel from my git.kernel.org nfsd-testing branch.
--
Chuck Lever
On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
> On 12/9/24 4:14 PM, Jeff Layton wrote:
> > Allow SETATTR to handle delegated timestamps. This patch assumes that
> > only the delegation holder has the ability to set the timestamps in this
> > way, so we allow this only if the SETATTR stateid refers to a
> > *_ATTRS_DELEG delegation.
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
> > fs/nfsd/nfs4state.c | 2 +-
> > fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
> > fs/nfsd/nfsd.h | 5 ++++-
> > 4 files changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > .na_iattr = &setattr->sa_iattr,
> > .na_seclabel = &setattr->sa_label,
> > };
> > + bool save_no_wcc, deleg_attrs;
> > + struct nfs4_stid *st = NULL;
> > struct inode *inode;
> > __be32 status = nfs_ok;
> > - bool save_no_wcc;
> > int err;
> >
> > - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
> > + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
> > + FATTR4_WORD2_TIME_DELEG_MODIFY);
> > +
> > + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
> > + int flags = WR_STATE;
> > +
> > + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
> > + flags |= RD_STATE;
> > +
> > status = nfs4_preprocess_stateid_op(rqstp, cstate,
> > &cstate->current_fh, &setattr->sa_stateid,
> > - WR_STATE, NULL, NULL);
> > + flags, NULL, &st);
> > if (status)
> > return status;
> > }
> > +
> > + if (deleg_attrs) {
> > + status = nfserr_bad_stateid;
> > + if (st->sc_type & SC_TYPE_DELEG) {
> > + struct nfs4_delegation *dp = delegstateid(st);
> > +
> > + /* Only for *_ATTRS_DELEG flavors */
> > + if (deleg_attrs_deleg(dp->dl_type))
> > + status = nfs_ok;
> > + }
> > + }
> > + if (st)
> > + nfs4_put_stid(st);
> > + if (status)
> > + return status;
> > +
> > err = fh_want_write(&cstate->current_fh);
> > if (err)
> > return nfserrno(err);
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> > static inline __be32
> > nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
> > {
> > - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
> > + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
> > return nfserr_openmode;
> > else
> > return nfs_ok;
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
> > *umask = mask & S_IRWXUGO;
> > iattr->ia_valid |= ATTR_MODE;
> > }
> > + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > + fattr4_time_deleg_access access;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
> > + return nfserr_bad_xdr;
> > + iattr->ia_atime.tv_sec = access.seconds;
> > + iattr->ia_atime.tv_nsec = access.nseconds;
> > + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
> > + }
> > + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > + fattr4_time_deleg_modify modify;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
> > + return nfserr_bad_xdr;
> > + iattr->ia_mtime.tv_sec = modify.seconds;
> > + iattr->ia_mtime.tv_nsec = modify.nseconds;
> > + iattr->ia_ctime.tv_sec = modify.seconds;
> > + iattr->ia_ctime.tv_nsec = modify.seconds;
> > + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
> > + }
> >
> > /* request sanity: did attrlist4 contain the expected number of words? */
> > if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
> > #endif
> > #define NFSD_WRITEABLE_ATTRS_WORD2 \
> > (FATTR4_WORD2_MODE_UMASK \
> > - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
> > + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
> > + | FATTR4_WORD2_TIME_DELEG_ACCESS \
> > + | FATTR4_WORD2_TIME_DELEG_MODIFY \
> > + )
> >
> > #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
> > NFSD_WRITEABLE_ATTRS_WORD0
> >
>
> Hi Jeff-
>
> After this patch is applied, I see failures of the git regression suite
> on NFSv4.2 mounts.
>
> Test Summary Report
> -------------------
> ./t3412-rebase-root.sh (Wstat: 256 (exited
> 1) Tests: 25 Failed: 5)
> Failed tests: 6, 19, 21-22, 24
> Non-zero exit status: 1
> ./t3400-rebase.sh (Wstat: 256 (exited
> 1) Tests: 38 Failed: 1)
> Failed test: 31
> Non-zero exit status: 1
> ./t3406-rebase-message.sh (Wstat: 256 (exited
> 1) Tests: 32 Failed: 2)
> Failed tests: 15, 20
> Non-zero exit status: 1
> ./t3428-rebase-signoff.sh (Wstat: 256 (exited
> 1) Tests: 7 Failed: 2)
> Failed tests: 6-7
> Non-zero exit status: 1
> ./t3418-rebase-continue.sh (Wstat: 256 (exited
> 1) Tests: 29 Failed: 1)
> Failed test: 7
> Non-zero exit status: 1
> ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
> 1) Tests: 27 Failed: 2)
> Failed tests: 3-4
> Non-zero exit status: 1
> ./t3404-rebase-interactive.sh (Wstat: 256 (exited
> 1) Tests: 131 Failed: 15)
> Failed tests: 32, 34-43, 45, 121-123
> Non-zero exit status: 1
> ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
> 1) Tests: 68 Failed: 1)
> Failed test: 34
> Non-zero exit status: 1
> ./t2013-checkout-submodule.sh (Wstat: 256 (exited
> 1) Tests: 74 Failed: 4)
> Failed tests: 26-27, 30-31
> Non-zero exit status: 1
> ./t5500-fetch-pack.sh (Wstat: 256 (exited
> 1) Tests: 375 Failed: 1)
> Failed test: 28
> Non-zero exit status: 1
> ./t5572-pull-submodule.sh (Wstat: 256 (exited
> 1) Tests: 67 Failed: 2)
> Failed tests: 5, 7
> Non-zero exit status: 1
> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
> Result: FAIL
>
> The NFS client and NFS server under test are running the same v6.13-rc2
> kernel from my git.kernel.org nfsd-testing branch.
>
>
I'm not seeing these failures. I ran the gitr suite under kdevops with
your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
All tests successful.
Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
Result: PASS
...and looking at the results of those specific tests, they did run and
they did pass.
I'm rerunning the tests now. It's possible the underlying fs matters.
Mine is exporting xfs. Yours?
--
Jeff Layton <jlayton@kernel.org>
On 12/13/24 9:14 AM, Jeff Layton wrote:
> On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
>> On 12/9/24 4:14 PM, Jeff Layton wrote:
>>> Allow SETATTR to handle delegated timestamps. This patch assumes that
>>> only the delegation holder has the ability to set the timestamps in this
>>> way, so we allow this only if the SETATTR stateid refers to a
>>> *_ATTRS_DELEG delegation.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
>>> fs/nfsd/nfs4state.c | 2 +-
>>> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
>>> fs/nfsd/nfsd.h | 5 ++++-
>>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>> index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
>>> --- a/fs/nfsd/nfs4proc.c
>>> +++ b/fs/nfsd/nfs4proc.c
>>> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>> .na_iattr = &setattr->sa_iattr,
>>> .na_seclabel = &setattr->sa_label,
>>> };
>>> + bool save_no_wcc, deleg_attrs;
>>> + struct nfs4_stid *st = NULL;
>>> struct inode *inode;
>>> __be32 status = nfs_ok;
>>> - bool save_no_wcc;
>>> int err;
>>>
>>> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
>>> + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
>>> + FATTR4_WORD2_TIME_DELEG_MODIFY);
>>> +
>>> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
>>> + int flags = WR_STATE;
>>> +
>>> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
>>> + flags |= RD_STATE;
>>> +
>>> status = nfs4_preprocess_stateid_op(rqstp, cstate,
>>> &cstate->current_fh, &setattr->sa_stateid,
>>> - WR_STATE, NULL, NULL);
>>> + flags, NULL, &st);
>>> if (status)
>>> return status;
>>> }
>>> +
>>> + if (deleg_attrs) {
>>> + status = nfserr_bad_stateid;
>>> + if (st->sc_type & SC_TYPE_DELEG) {
>>> + struct nfs4_delegation *dp = delegstateid(st);
>>> +
>>> + /* Only for *_ATTRS_DELEG flavors */
>>> + if (deleg_attrs_deleg(dp->dl_type))
>>> + status = nfs_ok;
>>> + }
>>> + }
>>> + if (st)
>>> + nfs4_put_stid(st);
>>> + if (status)
>>> + return status;
>>> +
>>> err = fh_want_write(&cstate->current_fh);
>>> if (err)
>>> return nfserrno(err);
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
>>> static inline __be32
>>> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
>>> {
>>> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
>>> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
>>> return nfserr_openmode;
>>> else
>>> return nfs_ok;
>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>> index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
>>> --- a/fs/nfsd/nfs4xdr.c
>>> +++ b/fs/nfsd/nfs4xdr.c
>>> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
>>> *umask = mask & S_IRWXUGO;
>>> iattr->ia_valid |= ATTR_MODE;
>>> }
>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>>> + fattr4_time_deleg_access access;
>>> +
>>> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
>>> + return nfserr_bad_xdr;
>>> + iattr->ia_atime.tv_sec = access.seconds;
>>> + iattr->ia_atime.tv_nsec = access.nseconds;
>>> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
>>> + }
>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
>>> + fattr4_time_deleg_modify modify;
>>> +
>>> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
>>> + return nfserr_bad_xdr;
>>> + iattr->ia_mtime.tv_sec = modify.seconds;
>>> + iattr->ia_mtime.tv_nsec = modify.nseconds;
>>> + iattr->ia_ctime.tv_sec = modify.seconds;
>>> + iattr->ia_ctime.tv_nsec = modify.seconds;
>>> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
>>> + }
>>>
>>> /* request sanity: did attrlist4 contain the expected number of words? */
>>> if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>> index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
>>> --- a/fs/nfsd/nfsd.h
>>> +++ b/fs/nfsd/nfsd.h
>>> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
>>> #endif
>>> #define NFSD_WRITEABLE_ATTRS_WORD2 \
>>> (FATTR4_WORD2_MODE_UMASK \
>>> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
>>> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>>> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
>>> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
>>> + )
>>>
>>> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
>>> NFSD_WRITEABLE_ATTRS_WORD0
>>>
>>
>> Hi Jeff-
>>
>> After this patch is applied, I see failures of the git regression suite
>> on NFSv4.2 mounts.
>>
>> Test Summary Report
>> -------------------
>> ./t3412-rebase-root.sh (Wstat: 256 (exited
>> 1) Tests: 25 Failed: 5)
>> Failed tests: 6, 19, 21-22, 24
>> Non-zero exit status: 1
>> ./t3400-rebase.sh (Wstat: 256 (exited
>> 1) Tests: 38 Failed: 1)
>> Failed test: 31
>> Non-zero exit status: 1
>> ./t3406-rebase-message.sh (Wstat: 256 (exited
>> 1) Tests: 32 Failed: 2)
>> Failed tests: 15, 20
>> Non-zero exit status: 1
>> ./t3428-rebase-signoff.sh (Wstat: 256 (exited
>> 1) Tests: 7 Failed: 2)
>> Failed tests: 6-7
>> Non-zero exit status: 1
>> ./t3418-rebase-continue.sh (Wstat: 256 (exited
>> 1) Tests: 29 Failed: 1)
>> Failed test: 7
>> Non-zero exit status: 1
>> ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
>> 1) Tests: 27 Failed: 2)
>> Failed tests: 3-4
>> Non-zero exit status: 1
>> ./t3404-rebase-interactive.sh (Wstat: 256 (exited
>> 1) Tests: 131 Failed: 15)
>> Failed tests: 32, 34-43, 45, 121-123
>> Non-zero exit status: 1
>> ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
>> 1) Tests: 68 Failed: 1)
>> Failed test: 34
>> Non-zero exit status: 1
>> ./t2013-checkout-submodule.sh (Wstat: 256 (exited
>> 1) Tests: 74 Failed: 4)
>> Failed tests: 26-27, 30-31
>> Non-zero exit status: 1
>> ./t5500-fetch-pack.sh (Wstat: 256 (exited
>> 1) Tests: 375 Failed: 1)
>> Failed test: 28
>> Non-zero exit status: 1
>> ./t5572-pull-submodule.sh (Wstat: 256 (exited
>> 1) Tests: 67 Failed: 2)
>> Failed tests: 5, 7
>> Non-zero exit status: 1
>> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
>> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
>> Result: FAIL
>>
>> The NFS client and NFS server under test are running the same v6.13-rc2
>> kernel from my git.kernel.org nfsd-testing branch.
>>
>>
>
> I'm not seeing these failures. I ran the gitr suite under kdevops with
> your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
>
> All tests successful.
> Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
> Result: PASS
>
> ...and looking at the results of those specific tests, they did run and
> they did pass.
>
> I'm rerunning the tests now. It's possible the underlying fs matters.
> Mine is exporting xfs. Yours?
Mine is btrfs, and the NFS version is v4.2 on TCP.
--
Chuck Lever
On Fri, 2024-12-13 at 09:18 -0500, Chuck Lever wrote:
> On 12/13/24 9:14 AM, Jeff Layton wrote:
> > On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
> > > On 12/9/24 4:14 PM, Jeff Layton wrote:
> > > > Allow SETATTR to handle delegated timestamps. This patch assumes that
> > > > only the delegation holder has the ability to set the timestamps in this
> > > > way, so we allow this only if the SETATTR stateid refers to a
> > > > *_ATTRS_DELEG delegation.
> > > >
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > > fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
> > > > fs/nfsd/nfs4state.c | 2 +-
> > > > fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
> > > > fs/nfsd/nfsd.h | 5 ++++-
> > > > 4 files changed, 53 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > > index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
> > > > --- a/fs/nfsd/nfs4proc.c
> > > > +++ b/fs/nfsd/nfs4proc.c
> > > > @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > > .na_iattr = &setattr->sa_iattr,
> > > > .na_seclabel = &setattr->sa_label,
> > > > };
> > > > + bool save_no_wcc, deleg_attrs;
> > > > + struct nfs4_stid *st = NULL;
> > > > struct inode *inode;
> > > > __be32 status = nfs_ok;
> > > > - bool save_no_wcc;
> > > > int err;
> > > >
> > > > - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
> > > > + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
> > > > + FATTR4_WORD2_TIME_DELEG_MODIFY);
> > > > +
> > > > + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
> > > > + int flags = WR_STATE;
> > > > +
> > > > + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
> > > > + flags |= RD_STATE;
> > > > +
> > > > status = nfs4_preprocess_stateid_op(rqstp, cstate,
> > > > &cstate->current_fh, &setattr->sa_stateid,
> > > > - WR_STATE, NULL, NULL);
> > > > + flags, NULL, &st);
> > > > if (status)
> > > > return status;
> > > > }
> > > > +
> > > > + if (deleg_attrs) {
> > > > + status = nfserr_bad_stateid;
> > > > + if (st->sc_type & SC_TYPE_DELEG) {
> > > > + struct nfs4_delegation *dp = delegstateid(st);
> > > > +
> > > > + /* Only for *_ATTRS_DELEG flavors */
> > > > + if (deleg_attrs_deleg(dp->dl_type))
> > > > + status = nfs_ok;
> > > > + }
> > > > + }
> > > > + if (st)
> > > > + nfs4_put_stid(st);
> > > > + if (status)
> > > > + return status;
> > > > +
> > > > err = fh_want_write(&cstate->current_fh);
> > > > if (err)
> > > > return nfserrno(err);
> > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > > index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
> > > > --- a/fs/nfsd/nfs4state.c
> > > > +++ b/fs/nfsd/nfs4state.c
> > > > @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> > > > static inline __be32
> > > > nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
> > > > {
> > > > - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
> > > > + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
> > > > return nfserr_openmode;
> > > > else
> > > > return nfs_ok;
> > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > > index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
> > > > --- a/fs/nfsd/nfs4xdr.c
> > > > +++ b/fs/nfsd/nfs4xdr.c
> > > > @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
> > > > *umask = mask & S_IRWXUGO;
> > > > iattr->ia_valid |= ATTR_MODE;
> > > > }
> > > > + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > > > + fattr4_time_deleg_access access;
> > > > +
> > > > + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
> > > > + return nfserr_bad_xdr;
> > > > + iattr->ia_atime.tv_sec = access.seconds;
> > > > + iattr->ia_atime.tv_nsec = access.nseconds;
> > > > + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
> > > > + }
> > > > + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > > > + fattr4_time_deleg_modify modify;
> > > > +
> > > > + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
> > > > + return nfserr_bad_xdr;
> > > > + iattr->ia_mtime.tv_sec = modify.seconds;
> > > > + iattr->ia_mtime.tv_nsec = modify.nseconds;
> > > > + iattr->ia_ctime.tv_sec = modify.seconds;
> > > > + iattr->ia_ctime.tv_nsec = modify.seconds;
> > > > + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
> > > > + }
> > > >
> > > > /* request sanity: did attrlist4 contain the expected number of words? */
> > > > if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
> > > > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > > > index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
> > > > --- a/fs/nfsd/nfsd.h
> > > > +++ b/fs/nfsd/nfsd.h
> > > > @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
> > > > #endif
> > > > #define NFSD_WRITEABLE_ATTRS_WORD2 \
> > > > (FATTR4_WORD2_MODE_UMASK \
> > > > - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
> > > > + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
> > > > + | FATTR4_WORD2_TIME_DELEG_ACCESS \
> > > > + | FATTR4_WORD2_TIME_DELEG_MODIFY \
> > > > + )
> > > >
> > > > #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
> > > > NFSD_WRITEABLE_ATTRS_WORD0
> > > >
> > >
> > > Hi Jeff-
> > >
> > > After this patch is applied, I see failures of the git regression suite
> > > on NFSv4.2 mounts.
> > >
> > > Test Summary Report
> > > -------------------
> > > ./t3412-rebase-root.sh (Wstat: 256 (exited
> > > 1) Tests: 25 Failed: 5)
> > > Failed tests: 6, 19, 21-22, 24
> > > Non-zero exit status: 1
> > > ./t3400-rebase.sh (Wstat: 256 (exited
> > > 1) Tests: 38 Failed: 1)
> > > Failed test: 31
> > > Non-zero exit status: 1
> > > ./t3406-rebase-message.sh (Wstat: 256 (exited
> > > 1) Tests: 32 Failed: 2)
> > > Failed tests: 15, 20
> > > Non-zero exit status: 1
> > > ./t3428-rebase-signoff.sh (Wstat: 256 (exited
> > > 1) Tests: 7 Failed: 2)
> > > Failed tests: 6-7
> > > Non-zero exit status: 1
> > > ./t3418-rebase-continue.sh (Wstat: 256 (exited
> > > 1) Tests: 29 Failed: 1)
> > > Failed test: 7
> > > Non-zero exit status: 1
> > > ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
> > > 1) Tests: 27 Failed: 2)
> > > Failed tests: 3-4
> > > Non-zero exit status: 1
> > > ./t3404-rebase-interactive.sh (Wstat: 256 (exited
> > > 1) Tests: 131 Failed: 15)
> > > Failed tests: 32, 34-43, 45, 121-123
> > > Non-zero exit status: 1
> > > ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
> > > 1) Tests: 68 Failed: 1)
> > > Failed test: 34
> > > Non-zero exit status: 1
> > > ./t2013-checkout-submodule.sh (Wstat: 256 (exited
> > > 1) Tests: 74 Failed: 4)
> > > Failed tests: 26-27, 30-31
> > > Non-zero exit status: 1
> > > ./t5500-fetch-pack.sh (Wstat: 256 (exited
> > > 1) Tests: 375 Failed: 1)
> > > Failed test: 28
> > > Non-zero exit status: 1
> > > ./t5572-pull-submodule.sh (Wstat: 256 (exited
> > > 1) Tests: 67 Failed: 2)
> > > Failed tests: 5, 7
> > > Non-zero exit status: 1
> > > Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
> > > 1037.05 cusr 6529.12 csys = 7587.52 CPU)
> > > Result: FAIL
> > >
> > > The NFS client and NFS server under test are running the same v6.13-rc2
> > > kernel from my git.kernel.org nfsd-testing branch.
> > >
> > >
> >
> > I'm not seeing these failures. I ran the gitr suite under kdevops with
> > your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
> >
> > All tests successful.
> > Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
> > Result: PASS
> >
> > ...and looking at the results of those specific tests, they did run and
> > they did pass.
> >
> > I'm rerunning the tests now. It's possible the underlying fs matters.
> > Mine is exporting xfs. Yours?
>
> Mine is btrfs, and the NFS version is v4.2 on TCP.
>
>
Nope, I still can't reproduce this with btrfs either. I'm also using
v4.2 on TCP. I assume you're running this under kdevops, so we should
have a relatively similar environment.
Are you also testing the same commit?
--
Jeff Layton <jlayton@kernel.org>
On 12/14/24 9:55 AM, Jeff Layton wrote:
> On Fri, 2024-12-13 at 09:18 -0500, Chuck Lever wrote:
>> On 12/13/24 9:14 AM, Jeff Layton wrote:
>>> On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
>>>> On 12/9/24 4:14 PM, Jeff Layton wrote:
>>>>> Allow SETATTR to handle delegated timestamps. This patch assumes that
>>>>> only the delegation holder has the ability to set the timestamps in this
>>>>> way, so we allow this only if the SETATTR stateid refers to a
>>>>> *_ATTRS_DELEG delegation.
>>>>>
>>>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>>>> ---
>>>>> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
>>>>> fs/nfsd/nfs4state.c | 2 +-
>>>>> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
>>>>> fs/nfsd/nfsd.h | 5 ++++-
>>>>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>> index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>> .na_iattr = &setattr->sa_iattr,
>>>>> .na_seclabel = &setattr->sa_label,
>>>>> };
>>>>> + bool save_no_wcc, deleg_attrs;
>>>>> + struct nfs4_stid *st = NULL;
>>>>> struct inode *inode;
>>>>> __be32 status = nfs_ok;
>>>>> - bool save_no_wcc;
>>>>> int err;
>>>>>
>>>>> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
>>>>> + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
>>>>> + FATTR4_WORD2_TIME_DELEG_MODIFY);
>>>>> +
>>>>> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
>>>>> + int flags = WR_STATE;
>>>>> +
>>>>> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
>>>>> + flags |= RD_STATE;
>>>>> +
>>>>> status = nfs4_preprocess_stateid_op(rqstp, cstate,
>>>>> &cstate->current_fh, &setattr->sa_stateid,
>>>>> - WR_STATE, NULL, NULL);
>>>>> + flags, NULL, &st);
>>>>> if (status)
>>>>> return status;
>>>>> }
>>>>> +
>>>>> + if (deleg_attrs) {
>>>>> + status = nfserr_bad_stateid;
>>>>> + if (st->sc_type & SC_TYPE_DELEG) {
>>>>> + struct nfs4_delegation *dp = delegstateid(st);
>>>>> +
>>>>> + /* Only for *_ATTRS_DELEG flavors */
>>>>> + if (deleg_attrs_deleg(dp->dl_type))
>>>>> + status = nfs_ok;
>>>>> + }
>>>>> + }
>>>>> + if (st)
>>>>> + nfs4_put_stid(st);
>>>>> + if (status)
>>>>> + return status;
>>>>> +
>>>>> err = fh_want_write(&cstate->current_fh);
>>>>> if (err)
>>>>> return nfserrno(err);
>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>>> index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
>>>>> --- a/fs/nfsd/nfs4state.c
>>>>> +++ b/fs/nfsd/nfs4state.c
>>>>> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
>>>>> static inline __be32
>>>>> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
>>>>> {
>>>>> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
>>>>> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
>>>>> return nfserr_openmode;
>>>>> else
>>>>> return nfs_ok;
>>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>>>> index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
>>>>> --- a/fs/nfsd/nfs4xdr.c
>>>>> +++ b/fs/nfsd/nfs4xdr.c
>>>>> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
>>>>> *umask = mask & S_IRWXUGO;
>>>>> iattr->ia_valid |= ATTR_MODE;
>>>>> }
>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>>>>> + fattr4_time_deleg_access access;
>>>>> +
>>>>> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
>>>>> + return nfserr_bad_xdr;
>>>>> + iattr->ia_atime.tv_sec = access.seconds;
>>>>> + iattr->ia_atime.tv_nsec = access.nseconds;
>>>>> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
>>>>> + }
>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
>>>>> + fattr4_time_deleg_modify modify;
>>>>> +
>>>>> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
>>>>> + return nfserr_bad_xdr;
>>>>> + iattr->ia_mtime.tv_sec = modify.seconds;
>>>>> + iattr->ia_mtime.tv_nsec = modify.nseconds;
>>>>> + iattr->ia_ctime.tv_sec = modify.seconds;
>>>>> + iattr->ia_ctime.tv_nsec = modify.seconds;
>>>>> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
>>>>> + }
>>>>>
>>>>> /* request sanity: did attrlist4 contain the expected number of words? */
>>>>> if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
>>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>>> index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
>>>>> --- a/fs/nfsd/nfsd.h
>>>>> +++ b/fs/nfsd/nfsd.h
>>>>> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
>>>>> #endif
>>>>> #define NFSD_WRITEABLE_ATTRS_WORD2 \
>>>>> (FATTR4_WORD2_MODE_UMASK \
>>>>> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
>>>>> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>>>>> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
>>>>> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
>>>>> + )
>>>>>
>>>>> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
>>>>> NFSD_WRITEABLE_ATTRS_WORD0
>>>>>
>>>>
>>>> Hi Jeff-
>>>>
>>>> After this patch is applied, I see failures of the git regression suite
>>>> on NFSv4.2 mounts.
>>>>
>>>> Test Summary Report
>>>> -------------------
>>>> ./t3412-rebase-root.sh (Wstat: 256 (exited
>>>> 1) Tests: 25 Failed: 5)
>>>> Failed tests: 6, 19, 21-22, 24
>>>> Non-zero exit status: 1
>>>> ./t3400-rebase.sh (Wstat: 256 (exited
>>>> 1) Tests: 38 Failed: 1)
>>>> Failed test: 31
>>>> Non-zero exit status: 1
>>>> ./t3406-rebase-message.sh (Wstat: 256 (exited
>>>> 1) Tests: 32 Failed: 2)
>>>> Failed tests: 15, 20
>>>> Non-zero exit status: 1
>>>> ./t3428-rebase-signoff.sh (Wstat: 256 (exited
>>>> 1) Tests: 7 Failed: 2)
>>>> Failed tests: 6-7
>>>> Non-zero exit status: 1
>>>> ./t3418-rebase-continue.sh (Wstat: 256 (exited
>>>> 1) Tests: 29 Failed: 1)
>>>> Failed test: 7
>>>> Non-zero exit status: 1
>>>> ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
>>>> 1) Tests: 27 Failed: 2)
>>>> Failed tests: 3-4
>>>> Non-zero exit status: 1
>>>> ./t3404-rebase-interactive.sh (Wstat: 256 (exited
>>>> 1) Tests: 131 Failed: 15)
>>>> Failed tests: 32, 34-43, 45, 121-123
>>>> Non-zero exit status: 1
>>>> ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 68 Failed: 1)
>>>> Failed test: 34
>>>> Non-zero exit status: 1
>>>> ./t2013-checkout-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 74 Failed: 4)
>>>> Failed tests: 26-27, 30-31
>>>> Non-zero exit status: 1
>>>> ./t5500-fetch-pack.sh (Wstat: 256 (exited
>>>> 1) Tests: 375 Failed: 1)
>>>> Failed test: 28
>>>> Non-zero exit status: 1
>>>> ./t5572-pull-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 67 Failed: 2)
>>>> Failed tests: 5, 7
>>>> Non-zero exit status: 1
>>>> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
>>>> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
>>>> Result: FAIL
>>>>
>>>> The NFS client and NFS server under test are running the same v6.13-rc2
>>>> kernel from my git.kernel.org nfsd-testing branch.
>>>>
>>>>
>>>
>>> I'm not seeing these failures. I ran the gitr suite under kdevops with
>>> your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
>>>
>>> All tests successful.
>>> Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
>>> Result: PASS
>>>
>>> ...and looking at the results of those specific tests, they did run and
>>> they did pass.
>>>
>>> I'm rerunning the tests now. It's possible the underlying fs matters.
>>> Mine is exporting xfs. Yours?
>>
>> Mine is btrfs, and the NFS version is v4.2 on TCP.
>>
>>
>
> Nope, I still can't reproduce this with btrfs either. I'm also using
> v4.2 on TCP. I assume you're running this under kdevops, so we should
> have a relatively similar environment.
I'm running the "stress" setting, which starts twice as many threads
as there are CPUs (so, 16, I think?). 32 nfsd threads.
> Are you also testing the same commit?
The first failing test run is on 6.13.0-rc2-00016-gb45eda1daa7d
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-testing&id=b45eda1daa7d79a2bf0426d27d4b359b8bb71d33
I'll take a closer look.
--
Chuck Lever
On 12/14/24 11:34 AM, Chuck Lever wrote:
> On 12/14/24 9:55 AM, Jeff Layton wrote:
>> On Fri, 2024-12-13 at 09:18 -0500, Chuck Lever wrote:
>>> On 12/13/24 9:14 AM, Jeff Layton wrote:
>>>> On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
>>>>> On 12/9/24 4:14 PM, Jeff Layton wrote:
>>>>>> Allow SETATTR to handle delegated timestamps. This patch assumes that
>>>>>> only the delegation holder has the ability to set the timestamps
>>>>>> in this
>>>>>> way, so we allow this only if the SETATTR stateid refers to a
>>>>>> *_ATTRS_DELEG delegation.
>>>>>>
>>>>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>>>>> ---
>>>>>> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
>>>>>> fs/nfsd/nfs4state.c | 2 +-
>>>>>> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
>>>>>> fs/nfsd/nfsd.h | 5 ++++-
>>>>>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>>> index
>>>>>> f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
>>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>>> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp,
>>>>>> struct nfsd4_compound_state *cstate,
>>>>>> .na_iattr = &setattr->sa_iattr,
>>>>>> .na_seclabel = &setattr->sa_label,
>>>>>> };
>>>>>> + bool save_no_wcc, deleg_attrs;
>>>>>> + struct nfs4_stid *st = NULL;
>>>>>> struct inode *inode;
>>>>>> __be32 status = nfs_ok;
>>>>>> - bool save_no_wcc;
>>>>>> int err;
>>>>>> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
>>>>>> + deleg_attrs = setattr->sa_bmval[2] &
>>>>>> (FATTR4_WORD2_TIME_DELEG_ACCESS |
>>>>>> + FATTR4_WORD2_TIME_DELEG_MODIFY);
>>>>>> +
>>>>>> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
>>>>>> + int flags = WR_STATE;
>>>>>> +
>>>>>> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
>>>>>> + flags |= RD_STATE;
>>>>>> +
>>>>>> status = nfs4_preprocess_stateid_op(rqstp, cstate,
>>>>>> &cstate->current_fh, &setattr->sa_stateid,
>>>>>> - WR_STATE, NULL, NULL);
>>>>>> + flags, NULL, &st);
>>>>>> if (status)
>>>>>> return status;
>>>>>> }
>>>>>> +
>>>>>> + if (deleg_attrs) {
>>>>>> + status = nfserr_bad_stateid;
>>>>>> + if (st->sc_type & SC_TYPE_DELEG) {
>>>>>> + struct nfs4_delegation *dp = delegstateid(st);
>>>>>> +
>>>>>> + /* Only for *_ATTRS_DELEG flavors */
>>>>>> + if (deleg_attrs_deleg(dp->dl_type))
>>>>>> + status = nfs_ok;
>>>>>> + }
>>>>>> + }
>>>>>> + if (st)
>>>>>> + nfs4_put_stid(st);
>>>>>> + if (status)
>>>>>> + return status;
>>>>>> +
>>>>>> err = fh_want_write(&cstate->current_fh);
>>>>>> if (err)
>>>>>> return nfserrno(err);
>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>>>> index
>>>>>> c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
>>>>>> --- a/fs/nfsd/nfs4state.c
>>>>>> +++ b/fs/nfsd/nfs4state.c
>>>>>> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct
>>>>>> nfsd4_compound_state *cstate,
>>>>>> static inline __be32
>>>>>> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
>>>>>> {
>>>>>> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
>>>>>> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
>>>>>> return nfserr_openmode;
>>>>>> else
>>>>>> return nfs_ok;
>>>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>>>>> index
>>>>>> 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
>>>>>> --- a/fs/nfsd/nfs4xdr.c
>>>>>> +++ b/fs/nfsd/nfs4xdr.c
>>>>>> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs
>>>>>> *argp, u32 *bmval, u32 bmlen,
>>>>>> *umask = mask & S_IRWXUGO;
>>>>>> iattr->ia_valid |= ATTR_MODE;
>>>>>> }
>>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>>>>>> + fattr4_time_deleg_access access;
>>>>>> +
>>>>>> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr,
>>>>>> &access))
>>>>>> + return nfserr_bad_xdr;
>>>>>> + iattr->ia_atime.tv_sec = access.seconds;
>>>>>> + iattr->ia_atime.tv_nsec = access.nseconds;
>>>>>> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
>>>>>> + }
>>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
>>>>>> + fattr4_time_deleg_modify modify;
>>>>>> +
>>>>>> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr,
>>>>>> &modify))
>>>>>> + return nfserr_bad_xdr;
>>>>>> + iattr->ia_mtime.tv_sec = modify.seconds;
>>>>>> + iattr->ia_mtime.tv_nsec = modify.nseconds;
>>>>>> + iattr->ia_ctime.tv_sec = modify.seconds;
>>>>>> + iattr->ia_ctime.tv_nsec = modify.seconds;
>>>>>> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME |
>>>>>> ATTR_MTIME_SET | ATTR_DELEG;
>>>>>> + }
>>>>>> /* request sanity: did attrlist4 contain the expected
>>>>>> number of words? */
>>>>>> if (attrlist4_count != xdr_stream_pos(argp->xdr) -
>>>>>> starting_pos)
>>>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>>>> index
>>>>>> 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
>>>>>> --- a/fs/nfsd/nfsd.h
>>>>>> +++ b/fs/nfsd/nfsd.h
>>>>>> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32
>>>>>> minorversion, const u32 *bmval)
>>>>>> #endif
>>>>>> #define NFSD_WRITEABLE_ATTRS_WORD2 \
>>>>>> (FATTR4_WORD2_MODE_UMASK \
>>>>>> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
>>>>>> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>>>>>> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
>>>>>> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
>>>>>> + )
>>>>>> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
>>>>>> NFSD_WRITEABLE_ATTRS_WORD0
>>>>>>
>>>>>
>>>>> Hi Jeff-
>>>>>
>>>>> After this patch is applied, I see failures of the git regression
>>>>> suite
>>>>> on NFSv4.2 mounts.
>>>>>
>>>>> Test Summary Report
>>>>> -------------------
>>>>> ./t3412-rebase-root.sh (Wstat: 256 (exited
>>>>> 1) Tests: 25 Failed: 5)
>>>>> Failed tests: 6, 19, 21-22, 24
>>>>> Non-zero exit status: 1
>>>>> ./t3400-rebase.sh (Wstat: 256 (exited
>>>>> 1) Tests: 38 Failed: 1)
>>>>> Failed test: 31
>>>>> Non-zero exit status: 1
>>>>> ./t3406-rebase-message.sh (Wstat: 256 (exited
>>>>> 1) Tests: 32 Failed: 2)
>>>>> Failed tests: 15, 20
>>>>> Non-zero exit status: 1
>>>>> ./t3428-rebase-signoff.sh (Wstat: 256 (exited
>>>>> 1) Tests: 7 Failed: 2)
>>>>> Failed tests: 6-7
>>>>> Non-zero exit status: 1
>>>>> ./t3418-rebase-continue.sh (Wstat: 256 (exited
>>>>> 1) Tests: 29 Failed: 1)
>>>>> Failed test: 7
>>>>> Non-zero exit status: 1
>>>>> ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
>>>>> 1) Tests: 27 Failed: 2)
>>>>> Failed tests: 3-4
>>>>> Non-zero exit status: 1
>>>>> ./t3404-rebase-interactive.sh (Wstat: 256 (exited
>>>>> 1) Tests: 131 Failed: 15)
>>>>> Failed tests: 32, 34-43, 45, 121-123
>>>>> Non-zero exit status: 1
>>>>> ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
>>>>> 1) Tests: 68 Failed: 1)
>>>>> Failed test: 34
>>>>> Non-zero exit status: 1
>>>>> ./t2013-checkout-submodule.sh (Wstat: 256 (exited
>>>>> 1) Tests: 74 Failed: 4)
>>>>> Failed tests: 26-27, 30-31
>>>>> Non-zero exit status: 1
>>>>> ./t5500-fetch-pack.sh (Wstat: 256 (exited
>>>>> 1) Tests: 375 Failed: 1)
>>>>> Failed test: 28
>>>>> Non-zero exit status: 1
>>>>> ./t5572-pull-submodule.sh (Wstat: 256 (exited
>>>>> 1) Tests: 67 Failed: 2)
>>>>> Failed tests: 5, 7
>>>>> Non-zero exit status: 1
>>>>> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
>>>>> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
>>>>> Result: FAIL
>>>>>
>>>>> The NFS client and NFS server under test are running the same
>>>>> v6.13-rc2
>>>>> kernel from my git.kernel.org nfsd-testing branch.
>>>>>
>>>>>
>>>>
>>>> I'm not seeing these failures. I ran the gitr suite under kdevops with
>>>> your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
>>>>
>>>> All tests successful.
>>>> Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys +
>>>> 1160.76 cusr 17870.80 csys = 19062.29 CPU)
>>>> Result: PASS
>>>>
>>>> ...and looking at the results of those specific tests, they did run and
>>>> they did pass.
>>>>
>>>> I'm rerunning the tests now. It's possible the underlying fs matters.
>>>> Mine is exporting xfs. Yours?
>>>
>>> Mine is btrfs, and the NFS version is v4.2 on TCP.
>>>
>>>
>>
>> Nope, I still can't reproduce this with btrfs either. I'm also using
>> v4.2 on TCP. I assume you're running this under kdevops, so we should
>> have a relatively similar environment.
>
> I'm running the "stress" setting, which starts twice as many threads
> as there are CPUs (so, 16, I think?). 32 nfsd threads.
Also, I'm testing 2.47.0 of git. The default version that kdevops uses
might be older.
>> Are you also testing the same commit?
>
> The first failing test run is on 6.13.0-rc2-00016-gb45eda1daa7d
>
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?
> h=nfsd-testing&id=b45eda1daa7d79a2bf0426d27d4b359b8bb71d33
>
> I'll take a closer look.
>
>
--
Chuck Lever
On 12/14/24 12:02 PM, Chuck Lever wrote:
> On 12/14/24 11:34 AM, Chuck Lever wrote:
>> On 12/14/24 9:55 AM, Jeff Layton wrote:
>>> On Fri, 2024-12-13 at 09:18 -0500, Chuck Lever wrote:
>>>> On 12/13/24 9:14 AM, Jeff Layton wrote:
>>>>> On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
>>>>>> On 12/9/24 4:14 PM, Jeff Layton wrote:
>>>>>>> Allow SETATTR to handle delegated timestamps. This patch assumes
>>>>>>> that
>>>>>>> only the delegation holder has the ability to set the timestamps
>>>>>>> in this
>>>>>>> way, so we allow this only if the SETATTR stateid refers to a
>>>>>>> *_ATTRS_DELEG delegation.
>>>>>>>
>>>>>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>>>>>> ---
>>>>>>> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
>>>>>>> fs/nfsd/nfs4state.c | 2 +-
>>>>>>> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
>>>>>>> fs/nfsd/nfsd.h | 5 ++++-
>>>>>>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>>>> index
>>>>>>> f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
>>>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>>>> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp,
>>>>>>> struct nfsd4_compound_state *cstate,
>>>>>>> .na_iattr = &setattr->sa_iattr,
>>>>>>> .na_seclabel = &setattr->sa_label,
>>>>>>> };
>>>>>>> + bool save_no_wcc, deleg_attrs;
>>>>>>> + struct nfs4_stid *st = NULL;
>>>>>>> struct inode *inode;
>>>>>>> __be32 status = nfs_ok;
>>>>>>> - bool save_no_wcc;
>>>>>>> int err;
>>>>>>> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
>>>>>>> + deleg_attrs = setattr->sa_bmval[2] &
>>>>>>> (FATTR4_WORD2_TIME_DELEG_ACCESS |
>>>>>>> + FATTR4_WORD2_TIME_DELEG_MODIFY);
>>>>>>> +
>>>>>>> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
>>>>>>> + int flags = WR_STATE;
>>>>>>> +
>>>>>>> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
>>>>>>> + flags |= RD_STATE;
>>>>>>> +
>>>>>>> status = nfs4_preprocess_stateid_op(rqstp, cstate,
>>>>>>> &cstate->current_fh, &setattr->sa_stateid,
>>>>>>> - WR_STATE, NULL, NULL);
>>>>>>> + flags, NULL, &st);
>>>>>>> if (status)
>>>>>>> return status;
>>>>>>> }
>>>>>>> +
>>>>>>> + if (deleg_attrs) {
>>>>>>> + status = nfserr_bad_stateid;
>>>>>>> + if (st->sc_type & SC_TYPE_DELEG) {
>>>>>>> + struct nfs4_delegation *dp = delegstateid(st);
>>>>>>> +
>>>>>>> + /* Only for *_ATTRS_DELEG flavors */
>>>>>>> + if (deleg_attrs_deleg(dp->dl_type))
>>>>>>> + status = nfs_ok;
>>>>>>> + }
>>>>>>> + }
>>>>>>> + if (st)
>>>>>>> + nfs4_put_stid(st);
>>>>>>> + if (status)
>>>>>>> + return status;
>>>>>>> +
>>>>>>> err = fh_want_write(&cstate->current_fh);
>>>>>>> if (err)
>>>>>>> return nfserrno(err);
>>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>>>>> index
>>>>>>> c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
>>>>>>> --- a/fs/nfsd/nfs4state.c
>>>>>>> +++ b/fs/nfsd/nfs4state.c
>>>>>>> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct
>>>>>>> nfsd4_compound_state *cstate,
>>>>>>> static inline __be32
>>>>>>> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
>>>>>>> {
>>>>>>> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
>>>>>>> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
>>>>>>> return nfserr_openmode;
>>>>>>> else
>>>>>>> return nfs_ok;
>>>>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>>>>>> index
>>>>>>> 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
>>>>>>> --- a/fs/nfsd/nfs4xdr.c
>>>>>>> +++ b/fs/nfsd/nfs4xdr.c
>>>>>>> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct
>>>>>>> nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
>>>>>>> *umask = mask & S_IRWXUGO;
>>>>>>> iattr->ia_valid |= ATTR_MODE;
>>>>>>> }
>>>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>>>>>>> + fattr4_time_deleg_access access;
>>>>>>> +
>>>>>>> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr,
>>>>>>> &access))
>>>>>>> + return nfserr_bad_xdr;
>>>>>>> + iattr->ia_atime.tv_sec = access.seconds;
>>>>>>> + iattr->ia_atime.tv_nsec = access.nseconds;
>>>>>>> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET |
>>>>>>> ATTR_DELEG;
>>>>>>> + }
>>>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
>>>>>>> + fattr4_time_deleg_modify modify;
>>>>>>> +
>>>>>>> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr,
>>>>>>> &modify))
>>>>>>> + return nfserr_bad_xdr;
>>>>>>> + iattr->ia_mtime.tv_sec = modify.seconds;
>>>>>>> + iattr->ia_mtime.tv_nsec = modify.nseconds;
>>>>>>> + iattr->ia_ctime.tv_sec = modify.seconds;
>>>>>>> + iattr->ia_ctime.tv_nsec = modify.seconds;
>>>>>>> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME |
>>>>>>> ATTR_MTIME_SET | ATTR_DELEG;
>>>>>>> + }
>>>>>>> /* request sanity: did attrlist4 contain the expected
>>>>>>> number of words? */
>>>>>>> if (attrlist4_count != xdr_stream_pos(argp->xdr) -
>>>>>>> starting_pos)
>>>>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>>>>> index
>>>>>>> 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
>>>>>>> --- a/fs/nfsd/nfsd.h
>>>>>>> +++ b/fs/nfsd/nfsd.h
>>>>>>> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32
>>>>>>> minorversion, const u32 *bmval)
>>>>>>> #endif
>>>>>>> #define NFSD_WRITEABLE_ATTRS_WORD2 \
>>>>>>> (FATTR4_WORD2_MODE_UMASK \
>>>>>>> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
>>>>>>> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>>>>>>> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
>>>>>>> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
>>>>>>> + )
>>>>>>> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
>>>>>>> NFSD_WRITEABLE_ATTRS_WORD0
>>>>>>>
>>>>>>
>>>>>> Hi Jeff-
>>>>>>
>>>>>> After this patch is applied, I see failures of the git regression
>>>>>> suite
>>>>>> on NFSv4.2 mounts.
>>>>>>
>>>>>> Test Summary Report
>>>>>> -------------------
>>>>>> ./t3412-rebase-root.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 25 Failed: 5)
>>>>>> Failed tests: 6, 19, 21-22, 24
>>>>>> Non-zero exit status: 1
>>>>>> ./t3400-rebase.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 38 Failed: 1)
>>>>>> Failed test: 31
>>>>>> Non-zero exit status: 1
>>>>>> ./t3406-rebase-message.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 32 Failed: 2)
>>>>>> Failed tests: 15, 20
>>>>>> Non-zero exit status: 1
>>>>>> ./t3428-rebase-signoff.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 7 Failed: 2)
>>>>>> Failed tests: 6-7
>>>>>> Non-zero exit status: 1
>>>>>> ./t3418-rebase-continue.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 29 Failed: 1)
>>>>>> Failed test: 7
>>>>>> Non-zero exit status: 1
>>>>>> ./t3415-rebase-autosquash.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 27 Failed: 2)
>>>>>> Failed tests: 3-4
>>>>>> Non-zero exit status: 1
>>>>>> ./t3404-rebase-interactive.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 131 Failed: 15)
>>>>>> Failed tests: 32, 34-43, 45, 121-123
>>>>>> Non-zero exit status: 1
>>>>>> ./t1013-read-tree-submodule.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 68 Failed: 1)
>>>>>> Failed test: 34
>>>>>> Non-zero exit status: 1
>>>>>> ./t2013-checkout-submodule.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 74 Failed: 4)
>>>>>> Failed tests: 26-27, 30-31
>>>>>> Non-zero exit status: 1
>>>>>> ./t5500-fetch-pack.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 375 Failed: 1)
>>>>>> Failed test: 28
>>>>>> Non-zero exit status: 1
>>>>>> ./t5572-pull-submodule.sh (Wstat: 256
>>>>>> (exited
>>>>>> 1) Tests: 67 Failed: 2)
>>>>>> Failed tests: 5, 7
>>>>>> Non-zero exit status: 1
>>>>>> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
>>>>>> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
>>>>>> Result: FAIL
>>>>>>
>>>>>> The NFS client and NFS server under test are running the same
>>>>>> v6.13-rc2
>>>>>> kernel from my git.kernel.org nfsd-testing branch.
>>>>>>
>>>>>>
>>>>>
>>>>> I'm not seeing these failures. I ran the gitr suite under kdevops with
>>>>> your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
>>>>>
>>>>> All tests successful.
>>>>> Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys
>>>>> + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
>>>>> Result: PASS
>>>>>
>>>>> ...and looking at the results of those specific tests, they did run
>>>>> and
>>>>> they did pass.
>>>>>
>>>>> I'm rerunning the tests now. It's possible the underlying fs matters.
>>>>> Mine is exporting xfs. Yours?
>>>>
>>>> Mine is btrfs, and the NFS version is v4.2 on TCP.
>>>>
>>>>
>>>
>>> Nope, I still can't reproduce this with btrfs either. I'm also using
>>> v4.2 on TCP. I assume you're running this under kdevops, so we should
>>> have a relatively similar environment.
>>
>> I'm running the "stress" setting, which starts twice as many threads
>> as there are CPUs (so, 16, I think?). 32 nfsd threads.
>
> Also, I'm testing 2.47.0 of git. The default version that kdevops uses
> might be older.
>
>
>>> Are you also testing the same commit?
>>
>> The first failing test run is on 6.13.0-rc2-00016-gb45eda1daa7d
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?
>> h=nfsd-testing&id=b45eda1daa7d79a2bf0426d27d4b359b8bb71d33
>>
>> I'll take a closer look.
My bad, I was testing with 2.46.0. When I changed to 2.47.0, the
failures in 9/10 went away.
Now with 2.47.0, I see one failure when testing in "stress" mode
on 10/10:
Test Summary Report
-------------------
./t4255-am-submodule.sh (Wstat: 256 (exited
1) Tests: 33 Failed: 1)
Failed test: 19
Non-zero exit status: 1
Files=1007, Tests=30810, 1330 wallclock secs (11.27 usr 9.81 sys +
1098.89 cusr 6274.49 csys = 7394.46 CPU)
Result: FAIL
It doesn't seem to reproduce in "fast" mode.
--
Chuck Lever
© 2016 - 2024 Red Hat, Inc.