Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
    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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux