linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/10] nfsd: implement the "delstid" draft
@ 2024-12-09 21:13 Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR Jeff Layton
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

We had a report from the kernel test robot that adding support for
OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION caused an "App Overhead"
regression in the fs_mark benchmark, and we dropped that series for
v6.13.

I've not been able to reproduce this problem. Even on the real hardware
to which I have access, I don't see the regression in App Overhead
values that the KTR is reporting in that test.xi

Here's the result of 10 runs of fs_mark with and without the last patch
applied. 375b976b5dbf is with that patch applied, and 08605b27815e is
without it:

               FSUse%      Count         Size      Files/sec     App Overhead

08605b27815e:     1        10240         4096        108.2         31292316
08605b27815e:     1        10240         4096        107.9         31287716
08605b27815e:     1        10240         4096        108.4         31196928
08605b27815e:     1        10240         4096        102.8         31356359
08605b27815e:     1        10240         4096        102.8         31406975
08605b27815e:     1        10240         4096        107.7         31089457
08605b27815e:     1        10240         4096        108.5         31177091
08605b27815e:     1        10240         4096        109.1         31169644
08605b27815e:     1        10240         4096        107.8         31192249
08605b27815e:     1        10240         4096        108.9         31073774

375b976b5dbf:     1        10240         4096        110.0         31741587
375b976b5dbf:     1        10240         4096        110.1         31602001
375b976b5dbf:     1        10240         4096        110.0         31833994
375b976b5dbf:     1        10240         4096        109.5         31737180
375b976b5dbf:     1        10240         4096        108.7         32027953
375b976b5dbf:     1        10240         4096        106.2         31625207
375b976b5dbf:     1        10240         4096        110.3         30842987
375b976b5dbf:     1        10240         4096        110.0         31664667
375b976b5dbf:     1        10240         4096        110.9         30925099
375b976b5dbf:     1        10240         4096        110.4         31247975

The difference is whether the last patch in this series is applied.
Adding them all up, I see about a ~0.9% regression in App overhead with
375b976b5dbf, but the Files/sec is ~2% faster in that case.

At this point, I'm not sure what to do. We don't have a clear definition
for what App Overhead represents, and I can't reproduce this. In my
testing with this, the performance of the part we care about seems to be
faster with this support enabled.

So, I'm posting what I have so far. This is a respin of that series
with a few minor changes. In particular, it should now be possible to
drop the last patch in the series, if that turns out to be the best way
to proceed.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v5:
- split out rework of SHARE_ACCESS_WANT flag masks into separate patch
- move WANT_OPEN_XOR_DELEGATION patch to the end of the series
- fix handling of OPEN4_SHARE_ACCESS_WANT_* values in nfsd4_deleg_xgrade_none_ext()
- Link to v4: https://lore.kernel.org/r/20241004-delstid-v4-0-62ac29c49c2e@kernel.org

Changes in v4:
- rebase onto Chuck's latest xdrgen patches
- ignore WANT_OPEN_XOR_DELEGATION flag if there is an extant open stateid
- consolidate patches that fix handling of delegated change attr
- Link to v3: https://lore.kernel.org/r/20240829-delstid-v3-0-271c60806c5d@kernel.org

Changes in v3:
- fix includes in nfs4xdr_gen.c
- drop ATTR_CTIME_DLG flag (just use ATTR_DELEG instead)
- proper handling for SETATTR (maybe? Outstanding q about stateid here)
- incorporate Neil's patches for handling non-delegation leases
- always return times from CB_GETATTR instead of from local vfs_getattr
- fix potential races vs. mgtimes by moving ctime handling into VFS layer
- Link to v2: https://lore.kernel.org/r/20240826-delstid-v2-0-e8ab5c0e39cc@kernel.org

Changes in v2:
- rebase onto Chuck's lkxdrgen branch, and reworked how autogenerated
  code is included
- declare nfsd_open_arguments as a global, so it doesn't have to be
  set up on the stack each time
- delegated timestamp support has been added
- Link to v1: https://lore.kernel.org/r/20240816-delstid-v1-0-c221c3dc14cd@kernel.org

---
Jeff Layton (10):
      nfsd: fix handling of delegated change attr in CB_GETATTR
      nfs_common: make include/linux/nfs4.h include generated nfs4_1.h
      nfsd: switch to autogenerated definitions for open_delegation_type4
      nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
      nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
      nfsd: add support for FATTR4_OPEN_ARGUMENTS
      nfsd: rework NFS4_SHARE_WANT_* flag handling
      nfsd: add support for delegated timestamps
      nfsd: handle delegated timestamps in SETATTR
      nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION

 Documentation/sunrpc/xdr/nfs4_1.x    | 186 +++++++++++++++++++++++++
 fs/nfsd/Makefile                     |  16 ++-
 fs/nfsd/nfs4callback.c               |  54 ++++++--
 fs/nfsd/nfs4proc.c                   |  31 ++++-
 fs/nfsd/nfs4state.c                  | 190 ++++++++++++++++++++------
 fs/nfsd/nfs4xdr.c                    | 121 ++++++++++++++---
 fs/nfsd/nfs4xdr_gen.c                | 256 +++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr_gen.h                |  25 ++++
 fs/nfsd/nfsd.h                       |  10 +-
 fs/nfsd/state.h                      |  18 +++
 fs/nfsd/xdr4cb.h                     |  10 +-
 include/linux/nfs4.h                 |   9 +-
 include/linux/nfs_xdr.h              |   5 -
 include/linux/sunrpc/xdrgen/nfs4_1.h | 153 +++++++++++++++++++++
 include/linux/time64.h               |   5 +
 include/uapi/linux/nfs4.h            |   7 +-
 16 files changed, 1005 insertions(+), 91 deletions(-)
---
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
change-id: 20241209-delstid-bae14ec4613c

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2025-01-19 15:19   ` Chuck Lever
  2024-12-09 21:13 ` [PATCH v5 02/10] nfs_common: make include/linux/nfs4.h include generated nfs4_1.h Jeff Layton
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

RFC8881, section 10.4.3 has some specific guidance as to how the
delegated change attribute should be handled. We currently don't follow
that guidance properly.

In particular, when the file is modified, the server always reports the
initial change attribute + 1. Section 10.4.3 however indicates that it
should be incremented on every GETATTR request from other clients.

Only request the change attribute until the file has been modified. If
there is an outstanding delegation, then increment the cached change
attribute on every GETATTR.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c |  8 +++++---
 fs/nfsd/nfs4xdr.c      | 15 +++++++++------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 3877b53e429fd89899d7dc35086bee8bda6eed07..25acb8624b854f5d0d184efec660e1f72cad8885 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -361,12 +361,14 @@ static void
 encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 			struct nfs4_cb_fattr *fattr)
 {
-	struct nfs4_delegation *dp =
-		container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
+	struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
 	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
+	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
 	u32 bmap[1];
 
-	bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE;
+	bmap[0] = FATTR4_WORD0_SIZE;
+	if (!ncf->ncf_file_modified)
+		bmap[0] |= FATTR4_WORD0_CHANGE;
 
 	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
 	encode_nfs_fh4(xdr, fh);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 53fac037611c05ff8ba2618f9e324a9cb54c3890..c8e8d3f0dff4bb5288186369aad821906e684db7 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2919,6 +2919,7 @@ struct nfsd4_fattr_args {
 	struct kstat		stat;
 	struct kstatfs		statfs;
 	struct nfs4_acl		*acl;
+	u64			change_attr;
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
 	void			*context;
 	int			contextlen;
@@ -3018,7 +3019,6 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
 					 const struct nfsd4_fattr_args *args)
 {
 	const struct svc_export *exp = args->exp;
-	u64 c;
 
 	if (unlikely(exp->ex_flags & NFSEXP_V4ROOT)) {
 		u32 flush_time = convert_to_wallclock(exp->cd->flush_time);
@@ -3029,9 +3029,7 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
 			return nfserr_resource;
 		return nfs_ok;
 	}
-
-	c = nfsd4_change_attribute(&args->stat);
-	return nfsd4_encode_changeid4(xdr, c);
+	return nfsd4_encode_changeid4(xdr, args->change_attr);
 }
 
 static __be32 nfsd4_encode_fattr4_size(struct xdr_stream *xdr,
@@ -3556,11 +3554,16 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	if (dp) {
 		struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
 
-		if (ncf->ncf_file_modified)
+		if (ncf->ncf_file_modified) {
+			++ncf->ncf_initial_cinfo;
 			args.stat.size = ncf->ncf_cur_fsize;
-
+		}
+		args.change_attr = ncf->ncf_initial_cinfo;
 		nfs4_put_stid(&dp->dl_stid);
+	} else {
+		args.change_attr = nfsd4_change_attribute(&args.stat);
 	}
+
 	if (err)
 		goto out_nfserr;
 

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 02/10] nfs_common: make include/linux/nfs4.h include generated nfs4_1.h
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 03/10] nfsd: switch to autogenerated definitions for open_delegation_type4 Jeff Layton
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

In the long run, the NFS development community intends to autogenerate a
lot of the XDR handling code.  Both the NFS client and server include
"include/linux/nfs4.hi". That file was hand-rolled, and some of the symbols
in it conflict with the autogenerated symbols.

Add a small nfs4_1.x to Documentation that currently just has the
necessary definitions for the delstid draft, and generate the relevant
header and source files. Make include/linux/nfs4.h include the generated
include/linux/sunrpc/xdrgen/nfs4_1.h and remove the conflicting
definitions from it and nfs_xdr.h.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 Documentation/sunrpc/xdr/nfs4_1.x    | 166 ++++++++++++++++++++++++
 fs/nfsd/Makefile                     |  16 ++-
 fs/nfsd/nfs4xdr_gen.c                | 239 +++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr_gen.h                |  25 ++++
 include/linux/nfs4.h                 |   7 +-
 include/linux/nfs_xdr.h              |   5 -
 include/linux/sunrpc/xdrgen/nfs4_1.h | 124 ++++++++++++++++++
 7 files changed, 570 insertions(+), 12 deletions(-)

diff --git a/Documentation/sunrpc/xdr/nfs4_1.x b/Documentation/sunrpc/xdr/nfs4_1.x
new file mode 100644
index 0000000000000000000000000000000000000000..fc37d1ecba0f40e46c6986df90d07a0e6e6ae9b2
--- /dev/null
+++ b/Documentation/sunrpc/xdr/nfs4_1.x
@@ -0,0 +1,166 @@
+/*
+ * Copyright (c) 2010 IETF Trust and the persons identified
+ * as the document authors.  All rights reserved.
+ *
+ * The document authors are identified in RFC 3530 and
+ * RFC 5661.
+ *
+ * Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the
+ * following conditions are met:
+ *
+ * - Redistributions of source code must retain the above
+ *   copyright notice, this list of conditions and the
+ *   following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the
+ *   following disclaimer in the documentation and/or other
+ *   materials provided with the distribution.
+ *
+ * - Neither the name of Internet Society, IETF or IETF
+ *   Trust, nor the names of specific contributors, may be
+ *   used to endorse or promote products derived from this
+ *   software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS
+ *   AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED
+ *   WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *   IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ *   FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO
+ *   EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ *   LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ *   EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ *   NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ *   SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *   INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ *   LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ *   OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
+ *   IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ *   ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+pragma header nfs4;
+
+/*
+ * Basic typedefs for RFC 1832 data type definitions
+ */
+typedef hyper		int64_t;
+typedef unsigned int	uint32_t;
+
+/*
+ * Basic data types
+ */
+typedef uint32_t	bitmap4<>;
+
+/*
+ * Timeval
+ */
+struct nfstime4 {
+	int64_t		seconds;
+	uint32_t	nseconds;
+};
+
+
+/*
+ * The following content was extracted from draft-ietf-nfsv4-delstid
+ */
+
+typedef bool            fattr4_offline;
+
+
+const FATTR4_OFFLINE            = 83;
+
+
+struct open_arguments4 {
+  bitmap4  oa_share_access;
+  bitmap4  oa_share_deny;
+  bitmap4  oa_share_access_want;
+  bitmap4  oa_open_claim;
+  bitmap4  oa_create_mode;
+};
+
+
+enum open_args_share_access4 {
+   OPEN_ARGS_SHARE_ACCESS_READ  = 1,
+   OPEN_ARGS_SHARE_ACCESS_WRITE = 2,
+   OPEN_ARGS_SHARE_ACCESS_BOTH  = 3
+};
+
+
+enum open_args_share_deny4 {
+   OPEN_ARGS_SHARE_DENY_NONE  = 0,
+   OPEN_ARGS_SHARE_DENY_READ  = 1,
+   OPEN_ARGS_SHARE_DENY_WRITE = 2,
+   OPEN_ARGS_SHARE_DENY_BOTH  = 3
+};
+
+
+enum open_args_share_access_want4 {
+   OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG           = 3,
+   OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG            = 4,
+   OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL              = 5,
+   OPEN_ARGS_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL
+                                                   = 17,
+   OPEN_ARGS_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED
+                                                   = 18,
+   OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS    = 20,
+   OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 21
+};
+
+
+enum open_args_open_claim4 {
+   OPEN_ARGS_OPEN_CLAIM_NULL          = 0,
+   OPEN_ARGS_OPEN_CLAIM_PREVIOUS      = 1,
+   OPEN_ARGS_OPEN_CLAIM_DELEGATE_CUR  = 2,
+   OPEN_ARGS_OPEN_CLAIM_DELEGATE_PREV = 3,
+   OPEN_ARGS_OPEN_CLAIM_FH            = 4,
+   OPEN_ARGS_OPEN_CLAIM_DELEG_CUR_FH  = 5,
+   OPEN_ARGS_OPEN_CLAIM_DELEG_PREV_FH = 6
+};
+
+
+enum open_args_createmode4 {
+   OPEN_ARGS_CREATEMODE_UNCHECKED4     = 0,
+   OPEN_ARGS_CREATE_MODE_GUARDED       = 1,
+   OPEN_ARGS_CREATEMODE_EXCLUSIVE4     = 2,
+   OPEN_ARGS_CREATE_MODE_EXCLUSIVE4_1  = 3
+};
+
+
+typedef open_arguments4 fattr4_open_arguments;
+pragma public fattr4_open_arguments;
+
+
+%/*
+% * Determine what OPEN supports.
+% */
+const FATTR4_OPEN_ARGUMENTS     = 86;
+
+
+const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
+
+
+const OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010;
+
+
+/*
+ * attributes for the delegation times being
+ * cached and served by the "client"
+ */
+typedef nfstime4        fattr4_time_deleg_access;
+typedef nfstime4        fattr4_time_deleg_modify;
+pragma public 		fattr4_time_deleg_access;
+pragma public		fattr4_time_deleg_modify;
+
+
+%/*
+% * New RECOMMENDED Attribute for
+% * delegation caching of times
+% */
+const FATTR4_TIME_DELEG_ACCESS  = 84;
+const FATTR4_TIME_DELEG_MODIFY  = 85;
+
+
+const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;
+
diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
index 18cbd3fa7691a27b39a3b1264637b9422aeb42ba..2f687619f65b38ddcfa0126aca69b6f98dd731f4 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -18,9 +18,23 @@ nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
 nfsd-$(CONFIG_NFSD_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4idmap.o \
-			   nfs4acl.o nfs4callback.o nfs4recover.o
+			   nfs4acl.o nfs4callback.o nfs4recover.o nfs4xdr_gen.o
 nfsd-$(CONFIG_NFSD_PNFS) += nfs4layouts.o
 nfsd-$(CONFIG_NFSD_BLOCKLAYOUT) += blocklayout.o blocklayoutxdr.o
 nfsd-$(CONFIG_NFSD_SCSILAYOUT) += blocklayout.o blocklayoutxdr.o
 nfsd-$(CONFIG_NFSD_FLEXFILELAYOUT) += flexfilelayout.o flexfilelayoutxdr.o
 nfsd-$(CONFIG_NFS_LOCALIO) += localio.o
+
+
+.PHONY: xdrgen
+
+xdrgen: ../../include/linux/sunrpc/xdrgen/nfs4_1.h nfs4xdr_gen.h nfs4xdr_gen.c
+
+../../include/linux/sunrpc/xdrgen/nfs4_1.h: ../../Documentation/sunrpc/xdr/nfs4_1.x
+	../../tools/net/sunrpc/xdrgen/xdrgen definitions $< > $@
+
+nfs4xdr_gen.h: ../../Documentation/sunrpc/xdr/nfs4_1.x
+	../../tools/net/sunrpc/xdrgen/xdrgen declarations $< > $@
+
+nfs4xdr_gen.c: ../../Documentation/sunrpc/xdr/nfs4_1.x
+	../../tools/net/sunrpc/xdrgen/xdrgen source $< > $@
diff --git a/fs/nfsd/nfs4xdr_gen.c b/fs/nfsd/nfs4xdr_gen.c
new file mode 100644
index 0000000000000000000000000000000000000000..e5d34f9a3147d9d51fb3b9db4c29b048b1083cbf
--- /dev/null
+++ b/fs/nfsd/nfs4xdr_gen.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0
+// Generated by xdrgen. Manual edits will be lost.
+// XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x
+// XDR specification modification time: Thu Oct  3 11:30:59 2024
+
+#include <linux/sunrpc/svc.h>
+
+#include "nfs4xdr_gen.h"
+
+static bool __maybe_unused
+xdrgen_decode_int64_t(struct xdr_stream *xdr, int64_t *ptr)
+{
+	return xdrgen_decode_hyper(xdr, ptr);
+};
+
+static bool __maybe_unused
+xdrgen_decode_uint32_t(struct xdr_stream *xdr, uint32_t *ptr)
+{
+	return xdrgen_decode_unsigned_int(xdr, ptr);
+};
+
+static bool __maybe_unused
+xdrgen_decode_bitmap4(struct xdr_stream *xdr, bitmap4 *ptr)
+{
+	if (xdr_stream_decode_u32(xdr, &ptr->count) < 0)
+		return false;
+	for (u32 i = 0; i < ptr->count; i++)
+		if (!xdrgen_decode_uint32_t(xdr, &ptr->element[i]))
+			return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_decode_nfstime4(struct xdr_stream *xdr, struct nfstime4 *ptr)
+{
+	if (!xdrgen_decode_int64_t(xdr, &ptr->seconds))
+		return false;
+	if (!xdrgen_decode_uint32_t(xdr, &ptr->nseconds))
+		return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_decode_fattr4_offline(struct xdr_stream *xdr, fattr4_offline *ptr)
+{
+	return xdrgen_decode_bool(xdr, ptr);
+};
+
+static bool __maybe_unused
+xdrgen_decode_open_arguments4(struct xdr_stream *xdr, struct open_arguments4 *ptr)
+{
+	if (!xdrgen_decode_bitmap4(xdr, &ptr->oa_share_access))
+		return false;
+	if (!xdrgen_decode_bitmap4(xdr, &ptr->oa_share_deny))
+		return false;
+	if (!xdrgen_decode_bitmap4(xdr, &ptr->oa_share_access_want))
+		return false;
+	if (!xdrgen_decode_bitmap4(xdr, &ptr->oa_open_claim))
+		return false;
+	if (!xdrgen_decode_bitmap4(xdr, &ptr->oa_create_mode))
+		return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_decode_open_args_share_access4(struct xdr_stream *xdr, open_args_share_access4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
+static bool __maybe_unused
+xdrgen_decode_open_args_share_deny4(struct xdr_stream *xdr, open_args_share_deny4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
+static bool __maybe_unused
+xdrgen_decode_open_args_share_access_want4(struct xdr_stream *xdr, open_args_share_access_want4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
+static bool __maybe_unused
+xdrgen_decode_open_args_open_claim4(struct xdr_stream *xdr, open_args_open_claim4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
+static bool __maybe_unused
+xdrgen_decode_open_args_createmode4(struct xdr_stream *xdr, open_args_createmode4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
+bool
+xdrgen_decode_fattr4_open_arguments(struct xdr_stream *xdr, fattr4_open_arguments *ptr)
+{
+	return xdrgen_decode_open_arguments4(xdr, ptr);
+};
+
+bool
+xdrgen_decode_fattr4_time_deleg_access(struct xdr_stream *xdr, fattr4_time_deleg_access *ptr)
+{
+	return xdrgen_decode_nfstime4(xdr, ptr);
+};
+
+bool
+xdrgen_decode_fattr4_time_deleg_modify(struct xdr_stream *xdr, fattr4_time_deleg_modify *ptr)
+{
+	return xdrgen_decode_nfstime4(xdr, ptr);
+};
+
+static bool __maybe_unused
+xdrgen_encode_int64_t(struct xdr_stream *xdr, const int64_t value)
+{
+	return xdrgen_encode_hyper(xdr, value);
+};
+
+static bool __maybe_unused
+xdrgen_encode_uint32_t(struct xdr_stream *xdr, const uint32_t value)
+{
+	return xdrgen_encode_unsigned_int(xdr, value);
+};
+
+static bool __maybe_unused
+xdrgen_encode_bitmap4(struct xdr_stream *xdr, const bitmap4 value)
+{
+	if (xdr_stream_encode_u32(xdr, value.count) != XDR_UNIT)
+		return false;
+	for (u32 i = 0; i < value.count; i++)
+		if (!xdrgen_encode_uint32_t(xdr, value.element[i]))
+			return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_encode_nfstime4(struct xdr_stream *xdr, const struct nfstime4 *value)
+{
+	if (!xdrgen_encode_int64_t(xdr, value->seconds))
+		return false;
+	if (!xdrgen_encode_uint32_t(xdr, value->nseconds))
+		return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_encode_fattr4_offline(struct xdr_stream *xdr, const fattr4_offline value)
+{
+	return xdrgen_encode_bool(xdr, value);
+};
+
+static bool __maybe_unused
+xdrgen_encode_open_arguments4(struct xdr_stream *xdr, const struct open_arguments4 *value)
+{
+	if (!xdrgen_encode_bitmap4(xdr, value->oa_share_access))
+		return false;
+	if (!xdrgen_encode_bitmap4(xdr, value->oa_share_deny))
+		return false;
+	if (!xdrgen_encode_bitmap4(xdr, value->oa_share_access_want))
+		return false;
+	if (!xdrgen_encode_bitmap4(xdr, value->oa_open_claim))
+		return false;
+	if (!xdrgen_encode_bitmap4(xdr, value->oa_create_mode))
+		return false;
+	return true;
+};
+
+static bool __maybe_unused
+xdrgen_encode_open_args_share_access4(struct xdr_stream *xdr, open_args_share_access4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
+
+static bool __maybe_unused
+xdrgen_encode_open_args_share_deny4(struct xdr_stream *xdr, open_args_share_deny4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
+
+static bool __maybe_unused
+xdrgen_encode_open_args_share_access_want4(struct xdr_stream *xdr, open_args_share_access_want4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
+
+static bool __maybe_unused
+xdrgen_encode_open_args_open_claim4(struct xdr_stream *xdr, open_args_open_claim4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
+
+static bool __maybe_unused
+xdrgen_encode_open_args_createmode4(struct xdr_stream *xdr, open_args_createmode4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
+
+bool
+xdrgen_encode_fattr4_open_arguments(struct xdr_stream *xdr, const fattr4_open_arguments *value)
+{
+	return xdrgen_encode_open_arguments4(xdr, value);
+};
+
+bool
+xdrgen_encode_fattr4_time_deleg_access(struct xdr_stream *xdr, const fattr4_time_deleg_access *value)
+{
+	return xdrgen_encode_nfstime4(xdr, value);
+};
+
+bool
+xdrgen_encode_fattr4_time_deleg_modify(struct xdr_stream *xdr, const fattr4_time_deleg_modify *value)
+{
+	return xdrgen_encode_nfstime4(xdr, value);
+};
diff --git a/fs/nfsd/nfs4xdr_gen.h b/fs/nfsd/nfs4xdr_gen.h
new file mode 100644
index 0000000000000000000000000000000000000000..c4c6a5075b17be3f931e2a20e282e33dc6e10ef1
--- /dev/null
+++ b/fs/nfsd/nfs4xdr_gen.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Generated by xdrgen. Manual edits will be lost. */
+/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
+/* XDR specification modification time: Thu Oct  3 11:30:59 2024 */
+
+#ifndef _LINUX_XDRGEN_NFS4_1_DECL_H
+#define _LINUX_XDRGEN_NFS4_1_DECL_H
+
+#include <linux/types.h>
+
+#include <linux/sunrpc/xdr.h>
+#include <linux/sunrpc/xdrgen/_defs.h>
+#include <linux/sunrpc/xdrgen/_builtins.h>
+#include <linux/sunrpc/xdrgen/nfs4_1.h>
+
+bool xdrgen_decode_fattr4_open_arguments(struct xdr_stream *xdr, fattr4_open_arguments *ptr);
+bool xdrgen_encode_fattr4_open_arguments(struct xdr_stream *xdr, const fattr4_open_arguments *value);
+
+bool xdrgen_decode_fattr4_time_deleg_access(struct xdr_stream *xdr, fattr4_time_deleg_access *ptr);
+bool xdrgen_encode_fattr4_time_deleg_access(struct xdr_stream *xdr, const fattr4_time_deleg_access *value);
+
+bool xdrgen_decode_fattr4_time_deleg_modify(struct xdr_stream *xdr, fattr4_time_deleg_modify *ptr);
+bool xdrgen_encode_fattr4_time_deleg_modify(struct xdr_stream *xdr, const fattr4_time_deleg_modify *value);
+
+#endif /* _LINUX_XDRGEN_NFS4_1_DECL_H */
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 8d7430d9f2183f08fc995b12370d0ddddf0bffba..b907192447755a614289554a01928c1ebb61c3dc 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -17,6 +17,7 @@
 #include <linux/uidgid.h>
 #include <uapi/linux/nfs4.h>
 #include <linux/sunrpc/msg_prot.h>
+#include <linux/sunrpc/xdrgen/nfs4_1.h>
 
 enum nfs4_acl_whotype {
 	NFS4_ACL_WHO_NAMED = 0,
@@ -512,12 +513,6 @@ enum {
 	FATTR4_XATTR_SUPPORT		= 82,
 };
 
-enum {
-	FATTR4_TIME_DELEG_ACCESS	= 84,
-	FATTR4_TIME_DELEG_MODIFY	= 85,
-	FATTR4_OPEN_ARGUMENTS		= 86,
-};
-
 /*
  * The following internal definitions enable processing the above
  * attribute bits within 32-bit word boundaries.
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 559273a0f16d81ef93b66c088cc0c5f86ab3109c..e74a87bb18a44be8b757039a0d527078034ced47 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1315,11 +1315,6 @@ struct nfs4_fsid_present_res {
 
 #endif /* CONFIG_NFS_V4 */
 
-struct nfstime4 {
-	u64	seconds;
-	u32	nseconds;
-};
-
 #ifdef CONFIG_NFS_V4_1
 
 struct pnfs_commit_bucket {
diff --git a/include/linux/sunrpc/xdrgen/nfs4_1.h b/include/linux/sunrpc/xdrgen/nfs4_1.h
new file mode 100644
index 0000000000000000000000000000000000000000..6025ab6b739833aad33567102e216c162003f408
--- /dev/null
+++ b/include/linux/sunrpc/xdrgen/nfs4_1.h
@@ -0,0 +1,124 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Generated by xdrgen. Manual edits will be lost. */
+/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
+/* XDR specification modification time: Thu Oct  3 11:30:59 2024 */
+
+#ifndef _LINUX_XDRGEN_NFS4_1_DEF_H
+#define _LINUX_XDRGEN_NFS4_1_DEF_H
+
+#include <linux/types.h>
+#include <linux/sunrpc/xdrgen/_defs.h>
+
+typedef s64 int64_t;
+
+typedef u32 uint32_t;
+
+typedef struct {
+	u32 count;
+	uint32_t *element;
+} bitmap4;
+
+struct nfstime4 {
+	int64_t seconds;
+	uint32_t nseconds;
+};
+
+typedef bool fattr4_offline;
+
+enum { FATTR4_OFFLINE = 83 };
+
+struct open_arguments4 {
+	bitmap4 oa_share_access;
+	bitmap4 oa_share_deny;
+	bitmap4 oa_share_access_want;
+	bitmap4 oa_open_claim;
+	bitmap4 oa_create_mode;
+};
+
+enum open_args_share_access4 {
+	OPEN_ARGS_SHARE_ACCESS_READ = 1,
+	OPEN_ARGS_SHARE_ACCESS_WRITE = 2,
+	OPEN_ARGS_SHARE_ACCESS_BOTH = 3,
+};
+typedef enum open_args_share_access4 open_args_share_access4;
+
+enum open_args_share_deny4 {
+	OPEN_ARGS_SHARE_DENY_NONE = 0,
+	OPEN_ARGS_SHARE_DENY_READ = 1,
+	OPEN_ARGS_SHARE_DENY_WRITE = 2,
+	OPEN_ARGS_SHARE_DENY_BOTH = 3,
+};
+typedef enum open_args_share_deny4 open_args_share_deny4;
+
+enum open_args_share_access_want4 {
+	OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG = 3,
+	OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG = 4,
+	OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL = 5,
+	OPEN_ARGS_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL = 17,
+	OPEN_ARGS_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED = 18,
+	OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 20,
+	OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 21,
+};
+typedef enum open_args_share_access_want4 open_args_share_access_want4;
+
+enum open_args_open_claim4 {
+	OPEN_ARGS_OPEN_CLAIM_NULL = 0,
+	OPEN_ARGS_OPEN_CLAIM_PREVIOUS = 1,
+	OPEN_ARGS_OPEN_CLAIM_DELEGATE_CUR = 2,
+	OPEN_ARGS_OPEN_CLAIM_DELEGATE_PREV = 3,
+	OPEN_ARGS_OPEN_CLAIM_FH = 4,
+	OPEN_ARGS_OPEN_CLAIM_DELEG_CUR_FH = 5,
+	OPEN_ARGS_OPEN_CLAIM_DELEG_PREV_FH = 6,
+};
+typedef enum open_args_open_claim4 open_args_open_claim4;
+
+enum open_args_createmode4 {
+	OPEN_ARGS_CREATEMODE_UNCHECKED4 = 0,
+	OPEN_ARGS_CREATE_MODE_GUARDED = 1,
+	OPEN_ARGS_CREATEMODE_EXCLUSIVE4 = 2,
+	OPEN_ARGS_CREATE_MODE_EXCLUSIVE4_1 = 3,
+};
+typedef enum open_args_createmode4 open_args_createmode4;
+
+typedef struct open_arguments4 fattr4_open_arguments;
+
+enum { FATTR4_OPEN_ARGUMENTS = 86 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000 };
+
+enum { OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010 };
+
+typedef struct nfstime4 fattr4_time_deleg_access;
+
+typedef struct nfstime4 fattr4_time_deleg_modify;
+
+enum { FATTR4_TIME_DELEG_ACCESS = 84 };
+
+enum { FATTR4_TIME_DELEG_MODIFY = 85 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
+
+#define NFS4_int64_t_sz                 \
+	(XDR_hyper)
+#define NFS4_uint32_t_sz                \
+	(XDR_unsigned_int)
+#define NFS4_bitmap4_sz                 (XDR_unsigned_int)
+#define NFS4_nfstime4_sz                \
+	(NFS4_int64_t_sz + NFS4_uint32_t_sz)
+#define NFS4_fattr4_offline_sz          \
+	(XDR_bool)
+#define NFS4_open_arguments4_sz         \
+	(NFS4_bitmap4_sz + NFS4_bitmap4_sz + NFS4_bitmap4_sz + NFS4_bitmap4_sz + NFS4_bitmap4_sz)
+#define NFS4_open_args_share_access4_sz (XDR_int)
+#define NFS4_open_args_share_deny4_sz   (XDR_int)
+#define NFS4_open_args_share_access_want4_sz (XDR_int)
+#define NFS4_open_args_open_claim4_sz   (XDR_int)
+#define NFS4_open_args_createmode4_sz   (XDR_int)
+#define NFS4_fattr4_open_arguments_sz   \
+	(NFS4_open_arguments4_sz)
+#define NFS4_fattr4_time_deleg_access_sz \
+	(NFS4_nfstime4_sz)
+#define NFS4_fattr4_time_deleg_modify_sz \
+	(NFS4_nfstime4_sz)
+
+#endif /* _LINUX_XDRGEN_NFS4_1_DEF_H */

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 03/10] nfsd: switch to autogenerated definitions for open_delegation_type4
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 02/10] nfs_common: make include/linux/nfs4.h include generated nfs4_1.h Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 04/10] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_* Jeff Layton
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Rename the enum with the same name in include/linux/nfs4.h, add the
proper enum to nfs4_1.x and regenerate the headers and source files.  Do
a mass rename of all NFS4_OPEN_DELEGATE_* to OPEN_DELEGATE_* in the nfsd
directory.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 Documentation/sunrpc/xdr/nfs4_1.x    |  9 ++++++++-
 fs/nfsd/nfs4state.c                  | 34 +++++++++++++++++-----------------
 fs/nfsd/nfs4xdr.c                    |  8 ++++----
 fs/nfsd/nfs4xdr_gen.c                | 19 ++++++++++++++++++-
 fs/nfsd/nfs4xdr_gen.h                |  2 +-
 include/linux/nfs4.h                 |  2 +-
 include/linux/sunrpc/xdrgen/nfs4_1.h | 13 ++++++++++++-
 7 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/Documentation/sunrpc/xdr/nfs4_1.x b/Documentation/sunrpc/xdr/nfs4_1.x
index fc37d1ecba0f40e46c6986df90d07a0e6e6ae9b2..ee9f8f249f1e71dbfc383007a6950ebc4104ed67 100644
--- a/Documentation/sunrpc/xdr/nfs4_1.x
+++ b/Documentation/sunrpc/xdr/nfs4_1.x
@@ -161,6 +161,13 @@ pragma public		fattr4_time_deleg_modify;
 const FATTR4_TIME_DELEG_ACCESS  = 84;
 const FATTR4_TIME_DELEG_MODIFY  = 85;
 
-
 const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;
 
+enum open_delegation_type4 {
+       OPEN_DELEGATE_NONE                  = 0,
+       OPEN_DELEGATE_READ                  = 1,
+       OPEN_DELEGATE_WRITE                 = 2,
+       OPEN_DELEGATE_NONE_EXT              = 3, /* new to v4.1 */
+       OPEN_DELEGATE_READ_ATTRS_DELEG      = 4,
+       OPEN_DELEGATE_WRITE_ATTRS_DELEG     = 5
+};
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 741b9449f727defc794347f1b116c955d715e691..c0e46ce0e068d8c73226dfe73adc58c24a630d77 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2887,7 +2887,7 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
 	seq_puts(s, ": { type: deleg, ");
 
 	seq_printf(s, "access: %s",
-		   ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
+		   ds->dl_type == OPEN_DELEGATE_READ ? "r" : "w");
 
 	/* XXX: lease time, whether it's being recalled. */
 
@@ -5472,7 +5472,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) && (dp->dl_type == NFS4_OPEN_DELEGATE_READ))
+	if ((flags & WR_STATE) && (dp->dl_type == OPEN_DELEGATE_READ))
 		return nfserr_openmode;
 	else
 		return nfs_ok;
@@ -5714,7 +5714,7 @@ static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
 		return NULL;
 	fl->fl_lmops = &nfsd_lease_mng_ops;
 	fl->c.flc_flags = FL_DELEG;
-	fl->c.flc_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
+	fl->c.flc_type = flag == OPEN_DELEGATE_READ ? F_RDLCK : F_WRLCK;
 	fl->c.flc_owner = (fl_owner_t)dp;
 	fl->c.flc_pid = current->tgid;
 	fl->c.flc_file = dp->dl_stid.sc_file->fi_deleg_file->nf_file;
@@ -5860,7 +5860,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	 */
 	if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
 		nf = find_rw_file(fp);
-		dl_type = NFS4_OPEN_DELEGATE_WRITE;
+		dl_type = OPEN_DELEGATE_WRITE;
 	}
 
 	/*
@@ -5869,7 +5869,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	 */
 	if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
 		nf = find_readable_file(fp);
-		dl_type = NFS4_OPEN_DELEGATE_READ;
+		dl_type = OPEN_DELEGATE_READ;
 	}
 
 	if (!nf)
@@ -5958,7 +5958,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 
 static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
 {
-	open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+	open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 	if (status == -EAGAIN)
 		open->op_why_no_deleg = WND4_CONTENTION;
 	else {
@@ -6074,20 +6074,20 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 			destroy_delegation(dp);
 			goto out_no_deleg;
 		}
-		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
+		open->op_delegate_type = OPEN_DELEGATE_WRITE;
 		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
 		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
 		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
 	} else {
-		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
+		open->op_delegate_type = OPEN_DELEGATE_READ;
 		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
 	}
 	nfs4_put_stid(&dp->dl_stid);
 	return;
 out_no_deleg:
-	open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE;
+	open->op_delegate_type = OPEN_DELEGATE_NONE;
 	if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
-	    open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
+	    open->op_delegate_type != OPEN_DELEGATE_NONE) {
 		dprintk("NFSD: WARNING: refusing delegation reclaim\n");
 		open->op_recall = true;
 	}
@@ -6102,17 +6102,17 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 					struct nfs4_delegation *dp)
 {
 	if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
-	    dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
-		open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+	    dp->dl_type == OPEN_DELEGATE_WRITE) {
+		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 		open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
 	} else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
-		   dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
-		open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+		   dp->dl_type == OPEN_DELEGATE_WRITE) {
+		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 		open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
 	}
 	/* Otherwise the client must be confused wanting a delegation
 	 * it already has, therefore we don't return
-	 * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
+	 * OPEN_DELEGATE_NONE_EXT and reason.
 	 */
 }
 
@@ -6202,7 +6202,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 
 	if (nfsd4_has_session(&resp->cstate)) {
 		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
-			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 			open->op_why_no_deleg = WND4_NOT_WANTED;
 			goto nodeleg;
 		}
@@ -6218,7 +6218,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	trace_nfsd_open(&stp->st_stid.sc_stateid);
 out:
 	/* 4.1 client trying to upgrade/downgrade delegation? */
-	if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
+	if (open->op_delegate_type == OPEN_DELEGATE_NONE && dp &&
 	    open->op_deleg_want)
 		nfsd4_deleg_xgrade_none_ext(open, dp);
 
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index c8e8d3f0dff4bb5288186369aad821906e684db7..593cf8c2ffe9dad90549ae0d0d5d9cbcbf18a690 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4232,18 +4232,18 @@ nfsd4_encode_open_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
 	if (xdr_stream_encode_u32(xdr, open->op_delegate_type) != XDR_UNIT)
 		return nfserr_resource;
 	switch (open->op_delegate_type) {
-	case NFS4_OPEN_DELEGATE_NONE:
+	case OPEN_DELEGATE_NONE:
 		status = nfs_ok;
 		break;
-	case NFS4_OPEN_DELEGATE_READ:
+	case OPEN_DELEGATE_READ:
 		/* read */
 		status = nfsd4_encode_open_read_delegation4(xdr, open);
 		break;
-	case NFS4_OPEN_DELEGATE_WRITE:
+	case OPEN_DELEGATE_WRITE:
 		/* write */
 		status = nfsd4_encode_open_write_delegation4(xdr, open);
 		break;
-	case NFS4_OPEN_DELEGATE_NONE_EXT:
+	case OPEN_DELEGATE_NONE_EXT:
 		/* od_whynone */
 		status = nfsd4_encode_open_none_delegation4(xdr, open);
 		break;
diff --git a/fs/nfsd/nfs4xdr_gen.c b/fs/nfsd/nfs4xdr_gen.c
index e5d34f9a3147d9d51fb3b9db4c29b048b1083cbf..a0e01f50a28d7f6828f3e6ef02f90b84bf180841 100644
--- a/fs/nfsd/nfs4xdr_gen.c
+++ b/fs/nfsd/nfs4xdr_gen.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 // Generated by xdrgen. Manual edits will be lost.
 // XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x
-// XDR specification modification time: Thu Oct  3 11:30:59 2024
+// XDR specification modification time: Sat Oct 12 08:10:54 2024
 
 #include <linux/sunrpc/svc.h>
 
@@ -135,6 +135,17 @@ xdrgen_decode_fattr4_time_deleg_modify(struct xdr_stream *xdr, fattr4_time_deleg
 	return xdrgen_decode_nfstime4(xdr, ptr);
 };
 
+static bool __maybe_unused
+xdrgen_decode_open_delegation_type4(struct xdr_stream *xdr, open_delegation_type4 *ptr)
+{
+	u32 val;
+
+	if (xdr_stream_decode_u32(xdr, &val) < 0)
+		return false;
+	*ptr = val;
+	return true;
+}
+
 static bool __maybe_unused
 xdrgen_encode_int64_t(struct xdr_stream *xdr, const int64_t value)
 {
@@ -237,3 +248,9 @@ xdrgen_encode_fattr4_time_deleg_modify(struct xdr_stream *xdr, const fattr4_time
 {
 	return xdrgen_encode_nfstime4(xdr, value);
 };
+
+static bool __maybe_unused
+xdrgen_encode_open_delegation_type4(struct xdr_stream *xdr, open_delegation_type4 value)
+{
+	return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
diff --git a/fs/nfsd/nfs4xdr_gen.h b/fs/nfsd/nfs4xdr_gen.h
index c4c6a5075b17be3f931e2a20e282e33dc6e10ef1..3fc8bde2b3b5db6f80f17b41e7f5991487cfa959 100644
--- a/fs/nfsd/nfs4xdr_gen.h
+++ b/fs/nfsd/nfs4xdr_gen.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Generated by xdrgen. Manual edits will be lost. */
 /* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Thu Oct  3 11:30:59 2024 */
+/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
 
 #ifndef _LINUX_XDRGEN_NFS4_1_DECL_H
 #define _LINUX_XDRGEN_NFS4_1_DECL_H
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index b907192447755a614289554a01928c1ebb61c3dc..71fbebfa43c7e2bd27708814c7300c506ce64c1b 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -366,7 +366,7 @@ enum limit_by4 {
 	NFS4_LIMIT_BLOCKS = 2
 };
 
-enum open_delegation_type4 {
+enum nfs4_open_delegation_type4 {
 	NFS4_OPEN_DELEGATE_NONE = 0,
 	NFS4_OPEN_DELEGATE_READ = 1,
 	NFS4_OPEN_DELEGATE_WRITE = 2,
diff --git a/include/linux/sunrpc/xdrgen/nfs4_1.h b/include/linux/sunrpc/xdrgen/nfs4_1.h
index 6025ab6b739833aad33567102e216c162003f408..9ca83a4a04cff8ebb5aafa08a24a2db771d6c1ef 100644
--- a/include/linux/sunrpc/xdrgen/nfs4_1.h
+++ b/include/linux/sunrpc/xdrgen/nfs4_1.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Generated by xdrgen. Manual edits will be lost. */
 /* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Thu Oct  3 11:30:59 2024 */
+/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
 
 #ifndef _LINUX_XDRGEN_NFS4_1_DEF_H
 #define _LINUX_XDRGEN_NFS4_1_DEF_H
@@ -98,6 +98,16 @@ enum { FATTR4_TIME_DELEG_MODIFY = 85 };
 
 enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
 
+enum open_delegation_type4 {
+	OPEN_DELEGATE_NONE = 0,
+	OPEN_DELEGATE_READ = 1,
+	OPEN_DELEGATE_WRITE = 2,
+	OPEN_DELEGATE_NONE_EXT = 3,
+	OPEN_DELEGATE_READ_ATTRS_DELEG = 4,
+	OPEN_DELEGATE_WRITE_ATTRS_DELEG = 5,
+};
+typedef enum open_delegation_type4 open_delegation_type4;
+
 #define NFS4_int64_t_sz                 \
 	(XDR_hyper)
 #define NFS4_uint32_t_sz                \
@@ -120,5 +130,6 @@ enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
 	(NFS4_nfstime4_sz)
 #define NFS4_fattr4_time_deleg_modify_sz \
 	(NFS4_nfstime4_sz)
+#define NFS4_open_delegation_type4_sz   (XDR_int)
 
 #endif /* _LINUX_XDRGEN_NFS4_1_DEF_H */

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 04/10] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (2 preceding siblings ...)
  2024-12-09 21:13 ` [PATCH v5 03/10] nfsd: switch to autogenerated definitions for open_delegation_type4 Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 05/10] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations Jeff Layton
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Add the OPEN4_SHARE_ACCESS_WANT constants from the nfs4.1 and delstid
draft into the nfs4_1.x file, and regenerate the headers and source
files. Do a mass renaming of NFS4_SHARE_WANT_* to
OPEN4_SHARE_ACCESS_WANT_* in the nfsd directory.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 Documentation/sunrpc/xdr/nfs4_1.x    | 15 ++++++++++++++-
 fs/nfsd/nfs4state.c                  | 16 ++++++++--------
 fs/nfsd/nfs4xdr.c                    | 12 ++++++------
 fs/nfsd/nfs4xdr_gen.c                |  2 +-
 fs/nfsd/nfs4xdr_gen.h                |  2 +-
 include/linux/sunrpc/xdrgen/nfs4_1.h | 24 +++++++++++++++++++++---
 6 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/Documentation/sunrpc/xdr/nfs4_1.x b/Documentation/sunrpc/xdr/nfs4_1.x
index ee9f8f249f1e71dbfc383007a6950ebc4104ed67..ca95150a3a29fc5418991bf2395326bd73645ea8 100644
--- a/Documentation/sunrpc/xdr/nfs4_1.x
+++ b/Documentation/sunrpc/xdr/nfs4_1.x
@@ -138,7 +138,6 @@ pragma public fattr4_open_arguments;
 const FATTR4_OPEN_ARGUMENTS     = 86;
 
 
-const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
 
 
 const OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010;
@@ -161,7 +160,21 @@ pragma public		fattr4_time_deleg_modify;
 const FATTR4_TIME_DELEG_ACCESS  = 84;
 const FATTR4_TIME_DELEG_MODIFY  = 85;
 
+
+
+/* new flags for share_access field of OPEN4args */
+const OPEN4_SHARE_ACCESS_WANT_DELEG_MASK        = 0xFF00;
+const OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE     = 0x0000;
+const OPEN4_SHARE_ACCESS_WANT_READ_DELEG        = 0x0100;
+const OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG       = 0x0200;
+const OPEN4_SHARE_ACCESS_WANT_ANY_DELEG         = 0x0300;
+const OPEN4_SHARE_ACCESS_WANT_NO_DELEG          = 0x0400;
+const OPEN4_SHARE_ACCESS_WANT_CANCEL            = 0x0500;
+
+const OPEN4_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL = 0x10000;
+const OPEN4_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED = 0x20000;
 const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;
+const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
 
 enum open_delegation_type4 {
        OPEN_DELEGATE_NONE                  = 0,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c0e46ce0e068d8c73226dfe73adc58c24a630d77..76b07c78559a0f59c0864b6247214f7136cd3dd2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5964,14 +5964,14 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
 	else {
 		open->op_why_no_deleg = WND4_RESOURCE;
 		switch (open->op_deleg_want) {
-		case NFS4_SHARE_WANT_READ_DELEG:
-		case NFS4_SHARE_WANT_WRITE_DELEG:
-		case NFS4_SHARE_WANT_ANY_DELEG:
+		case OPEN4_SHARE_ACCESS_WANT_READ_DELEG:
+		case OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG:
+		case OPEN4_SHARE_ACCESS_WANT_ANY_DELEG:
 			break;
-		case NFS4_SHARE_WANT_CANCEL:
+		case OPEN4_SHARE_ACCESS_WANT_CANCEL:
 			open->op_why_no_deleg = WND4_CANCELLED;
 			break;
-		case NFS4_SHARE_WANT_NO_DELEG:
+		case OPEN4_SHARE_ACCESS_WANT_NO_DELEG:
 			WARN_ON_ONCE(1);
 		}
 	}
@@ -6101,11 +6101,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 					struct nfs4_delegation *dp)
 {
-	if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
+	if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG &&
 	    dp->dl_type == OPEN_DELEGATE_WRITE) {
 		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 		open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
-	} else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
+	} else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG &&
 		   dp->dl_type == OPEN_DELEGATE_WRITE) {
 		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 		open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
@@ -6201,7 +6201,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	mutex_unlock(&stp->st_mutex);
 
 	if (nfsd4_has_session(&resp->cstate)) {
-		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
+		if (open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_NO_DELEG) {
 			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 			open->op_why_no_deleg = WND4_NOT_WANTED;
 			goto nodeleg;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 593cf8c2ffe9dad90549ae0d0d5d9cbcbf18a690..39a3b21bb90590f9f2711ca1cc0f44a68819d4a0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1067,12 +1067,12 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
 	if (!argp->minorversion)
 		return nfserr_bad_xdr;
 	switch (w & NFS4_SHARE_WANT_MASK) {
-	case NFS4_SHARE_WANT_NO_PREFERENCE:
-	case NFS4_SHARE_WANT_READ_DELEG:
-	case NFS4_SHARE_WANT_WRITE_DELEG:
-	case NFS4_SHARE_WANT_ANY_DELEG:
-	case NFS4_SHARE_WANT_NO_DELEG:
-	case NFS4_SHARE_WANT_CANCEL:
+	case OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE:
+	case OPEN4_SHARE_ACCESS_WANT_READ_DELEG:
+	case OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG:
+	case OPEN4_SHARE_ACCESS_WANT_ANY_DELEG:
+	case OPEN4_SHARE_ACCESS_WANT_NO_DELEG:
+	case OPEN4_SHARE_ACCESS_WANT_CANCEL:
 		break;
 	default:
 		return nfserr_bad_xdr;
diff --git a/fs/nfsd/nfs4xdr_gen.c b/fs/nfsd/nfs4xdr_gen.c
index a0e01f50a28d7f6828f3e6ef02f90b84bf180841..a17b5d8e60b3579caa2e2a8b40ed757070e1a622 100644
--- a/fs/nfsd/nfs4xdr_gen.c
+++ b/fs/nfsd/nfs4xdr_gen.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 // Generated by xdrgen. Manual edits will be lost.
 // XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x
-// XDR specification modification time: Sat Oct 12 08:10:54 2024
+// XDR specification modification time: Mon Oct 14 09:10:13 2024
 
 #include <linux/sunrpc/svc.h>
 
diff --git a/fs/nfsd/nfs4xdr_gen.h b/fs/nfsd/nfs4xdr_gen.h
index 3fc8bde2b3b5db6f80f17b41e7f5991487cfa959..41a0033b72562ee3c1fcdcd4a887ce635385b22b 100644
--- a/fs/nfsd/nfs4xdr_gen.h
+++ b/fs/nfsd/nfs4xdr_gen.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Generated by xdrgen. Manual edits will be lost. */
 /* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
+/* XDR specification modification time: Mon Oct 14 09:10:13 2024 */
 
 #ifndef _LINUX_XDRGEN_NFS4_1_DECL_H
 #define _LINUX_XDRGEN_NFS4_1_DECL_H
diff --git a/include/linux/sunrpc/xdrgen/nfs4_1.h b/include/linux/sunrpc/xdrgen/nfs4_1.h
index 9ca83a4a04cff8ebb5aafa08a24a2db771d6c1ef..cf21a14aa8850f4b21cd365cb7bc22a02c6097ce 100644
--- a/include/linux/sunrpc/xdrgen/nfs4_1.h
+++ b/include/linux/sunrpc/xdrgen/nfs4_1.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Generated by xdrgen. Manual edits will be lost. */
 /* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
+/* XDR specification modification time: Mon Oct 14 09:10:13 2024 */
 
 #ifndef _LINUX_XDRGEN_NFS4_1_DEF_H
 #define _LINUX_XDRGEN_NFS4_1_DEF_H
@@ -84,8 +84,6 @@ typedef struct open_arguments4 fattr4_open_arguments;
 
 enum { FATTR4_OPEN_ARGUMENTS = 86 };
 
-enum { OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000 };
-
 enum { OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010 };
 
 typedef struct nfstime4 fattr4_time_deleg_access;
@@ -96,8 +94,28 @@ enum { FATTR4_TIME_DELEG_ACCESS = 84 };
 
 enum { FATTR4_TIME_DELEG_MODIFY = 85 };
 
+enum { OPEN4_SHARE_ACCESS_WANT_DELEG_MASK = 0xFF00 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE = 0x0000 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_READ_DELEG = 0x0100 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG = 0x0200 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_ANY_DELEG = 0x0300 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_NO_DELEG = 0x0400 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_CANCEL = 0x0500 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL = 0x10000 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED = 0x20000 };
+
 enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
 
+enum { OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000 };
+
 enum open_delegation_type4 {
 	OPEN_DELEGATE_NONE = 0,
 	OPEN_DELEGATE_READ = 1,

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 05/10] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (3 preceding siblings ...)
  2024-12-09 21:13 ` [PATCH v5 04/10] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_* Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 06/10] nfsd: add support for FATTR4_OPEN_ARGUMENTS Jeff Layton
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Add some preparatory code to various functions that handle delegation
types to allow them to handle the OPEN_DELEGATE_*_ATTRS_DELEG constants.
Add helpers for detecting whether it's a read or write deleg, and
whether the attributes are delegated.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 43 ++++++++++++++++++++++++++++---------------
 fs/nfsd/nfs4xdr.c   |  2 ++
 fs/nfsd/state.h     | 16 ++++++++++++++++
 3 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 76b07c78559a0f59c0864b6247214f7136cd3dd2..b1e71462b9d91119457a60210a07021febedaf5c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2873,6 +2873,21 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
 	return 0;
 }
 
+static char *nfs4_show_deleg_type(u32 dl_type)
+{
+	switch (dl_type) {
+	case OPEN_DELEGATE_READ:
+		return "r";
+	case OPEN_DELEGATE_WRITE:
+		return "w";
+	case OPEN_DELEGATE_READ_ATTRS_DELEG:
+		return "ra";
+	case OPEN_DELEGATE_WRITE_ATTRS_DELEG:
+		return "wa";
+	}
+	return "?";
+}
+
 static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
 {
 	struct nfs4_delegation *ds;
@@ -2886,8 +2901,7 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
 	nfs4_show_stateid(s, &st->sc_stateid);
 	seq_puts(s, ": { type: deleg, ");
 
-	seq_printf(s, "access: %s",
-		   ds->dl_type == OPEN_DELEGATE_READ ? "r" : "w");
+	seq_printf(s, "access: %s", nfs4_show_deleg_type(ds->dl_type));
 
 	/* XXX: lease time, whether it's being recalled. */
 
@@ -5472,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) && (dp->dl_type == OPEN_DELEGATE_READ))
+	if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
 		return nfserr_openmode;
 	else
 		return nfs_ok;
@@ -5704,8 +5718,7 @@ static bool nfsd4_cb_channel_good(struct nfs4_client *clp)
 	return clp->cl_minorversion && clp->cl_cb_state == NFSD4_CB_UNKNOWN;
 }
 
-static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
-						int flag)
+static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp)
 {
 	struct file_lease *fl;
 
@@ -5714,7 +5727,7 @@ static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
 		return NULL;
 	fl->fl_lmops = &nfsd_lease_mng_ops;
 	fl->c.flc_flags = FL_DELEG;
-	fl->c.flc_type = flag == OPEN_DELEGATE_READ ? F_RDLCK : F_WRLCK;
+	fl->c.flc_type = deleg_is_read(dp->dl_type) ? F_RDLCK : F_WRLCK;
 	fl->c.flc_owner = (fl_owner_t)dp;
 	fl->c.flc_pid = current->tgid;
 	fl->c.flc_file = dp->dl_stid.sc_file->fi_deleg_file->nf_file;
@@ -5901,7 +5914,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	if (!dp)
 		goto out_delegees;
 
-	fl = nfs4_alloc_init_lease(dp, dl_type);
+	fl = nfs4_alloc_init_lease(dp);
 	if (!fl)
 		goto out_clnt_odstate;
 
@@ -6101,14 +6114,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 					struct nfs4_delegation *dp)
 {
-	if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG &&
-	    dp->dl_type == OPEN_DELEGATE_WRITE) {
-		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
-		open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
-	} else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG &&
-		   dp->dl_type == OPEN_DELEGATE_WRITE) {
-		open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
-		open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
+	if (deleg_is_write(dp->dl_type)) {
+		if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG) {
+			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
+			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
+		} else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) {
+			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
+			open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
+		}
 	}
 	/* Otherwise the client must be confused wanting a delegation
 	 * it already has, therefore we don't return
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 39a3b21bb90590f9f2711ca1cc0f44a68819d4a0..8c48da421a07bf460ace6eddc140ed5fedffd408 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4236,10 +4236,12 @@ nfsd4_encode_open_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
 		status = nfs_ok;
 		break;
 	case OPEN_DELEGATE_READ:
+	case OPEN_DELEGATE_READ_ATTRS_DELEG:
 		/* read */
 		status = nfsd4_encode_open_read_delegation4(xdr, open);
 		break;
 	case OPEN_DELEGATE_WRITE:
+	case OPEN_DELEGATE_WRITE_ATTRS_DELEG:
 		/* write */
 		status = nfsd4_encode_open_write_delegation4(xdr, open);
 		break;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e16bb3717fb9bb4725b9498c70dd3da72552845a..d8d7e568cf15e5cd84e00ed5548d164892ba7639 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -207,6 +207,22 @@ struct nfs4_delegation {
 	struct nfs4_cb_fattr    dl_cb_fattr;
 };
 
+static inline bool deleg_is_read(u32 dl_type)
+{
+	return (dl_type == OPEN_DELEGATE_READ || dl_type == OPEN_DELEGATE_READ_ATTRS_DELEG);
+}
+
+static inline bool deleg_is_write(u32 dl_type)
+{
+	return (dl_type == OPEN_DELEGATE_WRITE || dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG);
+}
+
+static inline bool deleg_attrs_deleg(u32 dl_type)
+{
+	return dl_type == OPEN_DELEGATE_READ_ATTRS_DELEG ||
+	       dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG;
+}
+
 #define cb_to_delegation(cb) \
 	container_of(cb, struct nfs4_delegation, dl_recall)
 

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 06/10] nfsd: add support for FATTR4_OPEN_ARGUMENTS
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (4 preceding siblings ...)
  2024-12-09 21:13 ` [PATCH v5 05/10] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:13 ` [PATCH v5 07/10] nfsd: rework NFS4_SHARE_WANT_* flag handling Jeff Layton
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Add support for FATTR4_OPEN_ARGUMENTS. This a new mechanism for the
client to discover what OPEN features the server supports.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsd.h    |  3 ++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 8c48da421a07bf460ace6eddc140ed5fedffd408..3006406aac8332e27ca9310288c724954f804e75 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -55,6 +55,7 @@
 #include "netns.h"
 #include "pnfs.h"
 #include "filecache.h"
+#include "nfs4xdr_gen.h"
 
 #include "trace.h"
 
@@ -3387,6 +3388,54 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 	return nfsd4_encode_bool(xdr, err == 0);
 }
 
+#define NFSD_OA_SHARE_ACCESS	(BIT(OPEN_ARGS_SHARE_ACCESS_READ)	| \
+				 BIT(OPEN_ARGS_SHARE_ACCESS_WRITE)	| \
+				 BIT(OPEN_ARGS_SHARE_ACCESS_BOTH))
+
+#define NFSD_OA_SHARE_DENY	(BIT(OPEN_ARGS_SHARE_DENY_NONE)		| \
+				 BIT(OPEN_ARGS_SHARE_DENY_READ)		| \
+				 BIT(OPEN_ARGS_SHARE_DENY_WRITE)	| \
+				 BIT(OPEN_ARGS_SHARE_DENY_BOTH))
+
+#define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL))
+
+#define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_PREVIOUS)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEGATE_CUR)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEGATE_PREV)| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_FH)		| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEG_CUR_FH)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEG_PREV_FH))
+
+#define NFSD_OA_CREATE_MODE	(BIT(OPEN_ARGS_CREATEMODE_UNCHECKED4)	| \
+				 BIT(OPEN_ARGS_CREATE_MODE_GUARDED)	| \
+				 BIT(OPEN_ARGS_CREATEMODE_EXCLUSIVE4)	| \
+				 BIT(OPEN_ARGS_CREATE_MODE_EXCLUSIVE4_1))
+
+static uint32_t oa_share_access = NFSD_OA_SHARE_ACCESS;
+static uint32_t oa_share_deny = NFSD_OA_SHARE_DENY;
+static uint32_t oa_share_access_want = NFSD_OA_SHARE_ACCESS_WANT;
+static uint32_t oa_open_claim = NFSD_OA_OPEN_CLAIM;
+static uint32_t oa_create_mode = NFSD_OA_CREATE_MODE;
+
+static const struct open_arguments4 nfsd_open_arguments = {
+	.oa_share_access = { .count = 1, .element = &oa_share_access },
+	.oa_share_deny = { .count = 1, .element = &oa_share_deny },
+	.oa_share_access_want = { .count = 1, .element = &oa_share_access_want },
+	.oa_open_claim = { .count = 1, .element = &oa_open_claim },
+	.oa_create_mode = { .count = 1, .element = &oa_create_mode },
+};
+
+static __be32 nfsd4_encode_fattr4_open_arguments(struct xdr_stream *xdr,
+						 const struct nfsd4_fattr_args *args)
+{
+	if (!xdrgen_encode_fattr4_open_arguments(xdr, &nfsd_open_arguments))
+		return nfserr_resource;
+	return nfs_ok;
+}
+
 static const nfsd4_enc_attr nfsd4_enc_fattr4_encode_ops[] = {
 	[FATTR4_SUPPORTED_ATTRS]	= nfsd4_encode_fattr4_supported_attrs,
 	[FATTR4_TYPE]			= nfsd4_encode_fattr4_type,
@@ -3487,6 +3536,7 @@ static const nfsd4_enc_attr nfsd4_enc_fattr4_encode_ops[] = {
 
 	[FATTR4_MODE_UMASK]		= nfsd4_encode_fattr4__noop,
 	[FATTR4_XATTR_SUPPORT]		= nfsd4_encode_fattr4_xattr_support,
+	[FATTR4_OPEN_ARGUMENTS]		= nfsd4_encode_fattr4_open_arguments,
 };
 
 /*
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 4b56ba1e8e48d08c4e3e52f378822c311193c3d4..1955c8e9c4c793728fa75dd136cadc735245483f 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -458,7 +458,8 @@ enum {
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
 	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS | \
-	FATTR4_WORD2_XATTR_SUPPORT)
+	FATTR4_WORD2_XATTR_SUPPORT | \
+	FATTR4_WORD2_OPEN_ARGUMENTS)
 
 extern const u32 nfsd_suppattrs[3][3];
 

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 07/10] nfsd: rework NFS4_SHARE_WANT_* flag handling
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (5 preceding siblings ...)
  2024-12-09 21:13 ` [PATCH v5 06/10] nfsd: add support for FATTR4_OPEN_ARGUMENTS Jeff Layton
@ 2024-12-09 21:13 ` Jeff Layton
  2024-12-09 21:14 ` [PATCH v5 08/10] nfsd: add support for delegated timestamps Jeff Layton
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:13 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

The delstid draft adds new NFS4_SHARE_WANT_TYPE_MASK values that don't
fit neatly into the existing WANT_MASK or WHEN_MASK. Add a new
NFS4_SHARE_WANT_MOD_MASK value and redefine NFS4_SHARE_WANT_MASK to
include it.

Also fix the checks in nfsd4_deleg_xgrade_none_ext() to check for the
flags instead of equality, since there may be modifier flags in the
value.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c       | 4 ++--
 fs/nfsd/nfs4xdr.c         | 2 +-
 include/uapi/linux/nfs4.h | 7 +++++--
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b1e71462b9d91119457a60210a07021febedaf5c..4c36e50b9eda119948461411ae8bc851907b2eb3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6115,10 +6115,10 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 					struct nfs4_delegation *dp)
 {
 	if (deleg_is_write(dp->dl_type)) {
-		if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG) {
+		if (open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_READ_DELEG) {
 			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 			open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
-		} else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) {
+		} else if (open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) {
 			open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
 			open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
 		}
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 3006406aac8332e27ca9310288c724954f804e75..7b867d0e5099608cc199c216a6140f3d45292376 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1067,7 +1067,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
 		return nfs_ok;
 	if (!argp->minorversion)
 		return nfserr_bad_xdr;
-	switch (w & NFS4_SHARE_WANT_MASK) {
+	switch (w & NFS4_SHARE_WANT_TYPE_MASK) {
 	case OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE:
 	case OPEN4_SHARE_ACCESS_WANT_READ_DELEG:
 	case OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG:
diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
index caf4db2fcbb94686631ec2232a8ff189c97c8617..4273e0249fcbb54996f5642f9920826b9d68b7b9 100644
--- a/include/uapi/linux/nfs4.h
+++ b/include/uapi/linux/nfs4.h
@@ -58,7 +58,7 @@
 #define NFS4_SHARE_DENY_BOTH	0x0003
 
 /* nfs41 */
-#define NFS4_SHARE_WANT_MASK		0xFF00
+#define NFS4_SHARE_WANT_TYPE_MASK	0xFF00
 #define NFS4_SHARE_WANT_NO_PREFERENCE	0x0000
 #define NFS4_SHARE_WANT_READ_DELEG	0x0100
 #define NFS4_SHARE_WANT_WRITE_DELEG	0x0200
@@ -66,13 +66,16 @@
 #define NFS4_SHARE_WANT_NO_DELEG	0x0400
 #define NFS4_SHARE_WANT_CANCEL		0x0500
 
-#define NFS4_SHARE_WHEN_MASK		0xF0000
+#define NFS4_SHARE_WHEN_MASK				0xF0000
 #define NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL	0x10000
 #define NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED		0x20000
 
+#define NFS4_SHARE_WANT_MOD_MASK			0xF00000
 #define NFS4_SHARE_WANT_DELEG_TIMESTAMPS		0x100000
 #define NFS4_SHARE_WANT_OPEN_XOR_DELEGATION		0x200000
 
+#define NFS4_SHARE_WANT_MASK	(NFS4_SHARE_WANT_TYPE_MASK | NFS4_SHARE_WANT_MOD_MASK)
+
 #define NFS4_CDFC4_FORE	0x1
 #define NFS4_CDFC4_BACK 0x2
 #define NFS4_CDFC4_BOTH 0x3

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 08/10] nfsd: add support for delegated timestamps
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (6 preceding siblings ...)
  2024-12-09 21:13 ` [PATCH v5 07/10] nfsd: rework NFS4_SHARE_WANT_* flag handling Jeff Layton
@ 2024-12-09 21:14 ` Jeff Layton
  2024-12-09 21:14 ` [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR Jeff Layton
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:14 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Add support for the delegated timestamps on write delegations. This
allows the server to proxy timestamps from the delegation holder to
other clients that are doing GETATTRs vs. the same inode.

When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.

Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
those. Vet those timestamps according to the delstid spec and update
the inode attrs if necessary.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
 fs/nfsd/nfs4state.c    | 99 +++++++++++++++++++++++++++++++++++++++++++-------
 fs/nfsd/nfs4xdr.c      | 15 +++++++-
 fs/nfsd/nfsd.h         |  2 +
 fs/nfsd/state.h        |  2 +
 fs/nfsd/xdr4cb.h       | 10 +++--
 include/linux/time64.h |  5 +++
 7 files changed, 152 insertions(+), 23 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 25acb8624b854f5d0d184efec660e1f72cad8885..0d1ebeaca14a40980d08e223cb6598e20c6ce21a 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -42,6 +42,7 @@
 #include "trace.h"
 #include "xdr4cb.h"
 #include "xdr4.h"
+#include "nfs4xdr_gen.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 {
 	fattr->ncf_cb_change = 0;
 	fattr->ncf_cb_fsize = 0;
+	fattr->ncf_cb_atime.tv_sec = 0;
+	fattr->ncf_cb_atime.tv_nsec = 0;
+	fattr->ncf_cb_mtime.tv_sec = 0;
+	fattr->ncf_cb_mtime.tv_nsec = 0;
+
 	if (bitmap[0] & FATTR4_WORD0_CHANGE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
 			return -NFSERR_BAD_XDR;
 	if (bitmap[0] & FATTR4_WORD0_SIZE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
 			return -NFSERR_BAD_XDR;
+	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+		fattr4_time_deleg_access access;
+
+		if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
+			return -NFSERR_BAD_XDR;
+		fattr->ncf_cb_atime.tv_sec = access.seconds;
+		fattr->ncf_cb_atime.tv_nsec = access.nseconds;
+
+	}
+	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+		fattr4_time_deleg_modify modify;
+
+		if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
+			return -NFSERR_BAD_XDR;
+		fattr->ncf_cb_mtime.tv_sec = modify.seconds;
+		fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
+
+	}
 	return 0;
 }
 
@@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 	struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
 	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
 	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
-	u32 bmap[1];
+	u32 bmap_size = 1;
+	u32 bmap[3];
 
 	bmap[0] = FATTR4_WORD0_SIZE;
 	if (!ncf->ncf_file_modified)
 		bmap[0] |= FATTR4_WORD0_CHANGE;
 
+	if (deleg_attrs_deleg(dp->dl_type)) {
+		bmap[1] = 0;
+		bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
+		bmap_size = 3;
+	}
 	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
 	encode_nfs_fh4(xdr, fh);
-	encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
+	encode_bitmap4(xdr, bmap, bmap_size);
 	hdr->nops++;
 }
 
@@ -636,7 +666,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
 	struct nfs4_cb_compound_hdr hdr;
 	int status;
 	u32 bitmap[3] = {0};
-	u32 attrlen;
+	u32 attrlen, maxlen;
 	struct nfs4_cb_fattr *ncf =
 		container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
 
@@ -655,7 +685,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
 		return -NFSERR_BAD_XDR;
 	if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
 		return -NFSERR_BAD_XDR;
-	if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
+	maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
+	if (bitmap[2] != 0)
+		maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
+			   sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
+	if (attrlen > maxlen)
 		return -NFSERR_BAD_XDR;
 	status = decode_cb_fattr4(xdr, bitmap, ncf);
 	return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4c36e50b9eda119948461411ae8bc851907b2eb3..c882eeba7830b0249ccd74654f81e63b12a30f14 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5842,13 +5842,14 @@ static struct nfs4_delegation *
 nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 		    struct svc_fh *parent)
 {
-	int status = 0;
+	bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
 	struct nfs4_client *clp = stp->st_stid.sc_client;
 	struct nfs4_file *fp = stp->st_stid.sc_file;
 	struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
 	struct nfs4_delegation *dp;
 	struct nfsd_file *nf = NULL;
 	struct file_lease *fl;
+	int status = 0;
 	u32 dl_type;
 
 	/*
@@ -5873,7 +5874,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	 */
 	if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
 		nf = find_rw_file(fp);
-		dl_type = OPEN_DELEGATE_WRITE;
+		dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
 	}
 
 	/*
@@ -5882,7 +5883,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	 */
 	if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
 		nf = find_readable_file(fp);
-		dl_type = OPEN_DELEGATE_READ;
+		dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
 	}
 
 	if (!nf)
@@ -6040,13 +6041,14 @@ static void
 nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 		     struct svc_fh *currentfh)
 {
-	struct nfs4_delegation *dp;
+	bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
 	struct nfs4_openowner *oo = openowner(stp->st_stateowner);
 	struct nfs4_client *clp = stp->st_stid.sc_client;
 	struct svc_fh *parent = NULL;
-	int cb_up;
-	int status = 0;
+	struct nfs4_delegation *dp;
 	struct kstat stat;
+	int status = 0;
+	int cb_up;
 
 	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
 	open->op_recall = false;
@@ -6087,12 +6089,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 			destroy_delegation(dp);
 			goto out_no_deleg;
 		}
-		open->op_delegate_type = OPEN_DELEGATE_WRITE;
+		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
+						    OPEN_DELEGATE_WRITE;
 		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
 		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
 		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
 	} else {
-		open->op_delegate_type = OPEN_DELEGATE_READ;
+		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
+						    OPEN_DELEGATE_READ;
 		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
 	}
 	nfs4_put_stid(&dp->dl_stid);
@@ -8901,6 +8905,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
 	get_stateid(cstate, &u->write.wr_stateid);
 }
 
+/**
+ * set_cb_time - vet and set the timespec for a cb_getattr update
+ * @cb: timestamp from the CB_GETATTR response
+ * @orig: original timestamp in the inode
+ * @now: current time
+ *
+ * Given a timestamp in a CB_GETATTR response, check it against the
+ * current timestamp in the inode and the current time. Returns true
+ * if the inode's timestamp needs to be updated, and false otherwise.
+ * @cb may also be changed if the timestamp needs to be clamped.
+ */
+static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
+			const struct timespec64 *now)
+{
+
+	/*
+	 * "When the time presented is before the original time, then the
+	 *  update is ignored." Also no need to update if there is no change.
+	 */
+	if (timespec64_compare(cb, orig) <= 0)
+		return false;
+
+	/*
+	 * "When the time presented is in the future, the server can either
+	 *  clamp the new time to the current time, or it may
+	 *  return NFS4ERR_DELAY to the client, allowing it to retry."
+	 */
+	if (timespec64_compare(cb, now) > 0) {
+		/* clamp it */
+		*cb = *now;
+	}
+
+	return true;
+}
+
+static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
+{
+	struct inode *inode = d_inode(dentry);
+	struct timespec64 now = current_time(inode);
+	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
+	struct iattr attrs = { };
+	int ret;
+
+	if (deleg_attrs_deleg(dp->dl_type)) {
+		struct timespec64 atime = inode_get_atime(inode);
+		struct timespec64 mtime = inode_get_mtime(inode);
+
+		attrs.ia_atime = ncf->ncf_cb_atime;
+		attrs.ia_mtime = ncf->ncf_cb_mtime;
+
+		if (set_cb_time(&attrs.ia_atime, &atime, &now))
+			attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
+
+		if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
+			attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
+			attrs.ia_ctime = attrs.ia_mtime;
+		}
+	} else {
+		attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
+		attrs.ia_mtime = attrs.ia_ctime = now;
+	}
+
+	if (!attrs.ia_valid)
+		return 0;
+
+	attrs.ia_valid |= ATTR_DELEG;
+	inode_lock(inode);
+	ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
+	inode_unlock(inode);
+	return ret;
+}
+
 /**
  * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
  * @rqstp: RPC transaction context
@@ -8927,7 +9003,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 	struct file_lock_context *ctx;
 	struct nfs4_delegation *dp = NULL;
 	struct file_lease *fl;
-	struct iattr attrs;
 	struct nfs4_cb_fattr *ncf;
 	struct inode *inode = d_inode(dentry);
 
@@ -8989,11 +9064,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 		 * not update the file's metadata with the client's
 		 * modified size
 		 */
-		attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
-		attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
-		inode_lock(inode);
-		err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
-		inode_unlock(inode);
+		err = cb_getattr_update_times(dentry, dp);
 		if (err) {
 			status = nfserrno(err);
 			goto out_status;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 7b867d0e5099608cc199c216a6140f3d45292376..0561c99b5def2eccf679bf3ea0e5b1a57d5d8374 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3399,7 +3399,8 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 
 #define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
-					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL))
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS))
 
 #define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
 				 BIT(OPEN_ARGS_OPEN_CLAIM_PREVIOUS)	| \
@@ -3592,7 +3593,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		if (status)
 			goto out;
 	}
-	if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
+	if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
+			    FATTR4_WORD0_SIZE)) ||
+	    (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
+			    FATTR4_WORD1_TIME_MODIFY |
+			    FATTR4_WORD1_TIME_METADATA))) {
 		status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
 		if (status)
 			goto out;
@@ -3607,8 +3612,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		if (ncf->ncf_file_modified) {
 			++ncf->ncf_initial_cinfo;
 			args.stat.size = ncf->ncf_cur_fsize;
+			if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
+				args.stat.mtime = ncf->ncf_cb_mtime;
 		}
 		args.change_attr = ncf->ncf_initial_cinfo;
+
+		if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
+			args.stat.atime = ncf->ncf_cb_atime;
+
 		nfs4_put_stid(&dp->dl_stid);
 	} else {
 		args.change_attr = nfsd4_change_attribute(&args.stat);
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -459,6 +459,8 @@ enum {
 	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS | \
 	FATTR4_WORD2_XATTR_SUPPORT | \
+	FATTR4_WORD2_TIME_DELEG_ACCESS | \
+	FATTR4_WORD2_TIME_DELEG_MODIFY | \
 	FATTR4_WORD2_OPEN_ARGUMENTS)
 
 extern const u32 nfsd_suppattrs[3][3];
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d8d7e568cf15e5cd84e00ed5548d164892ba7639..554041da8593e14987439cb1c152d1a072bd00ad 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -159,6 +159,8 @@ struct nfs4_cb_fattr {
 	/* from CB_GETATTR reply */
 	u64 ncf_cb_change;
 	u64 ncf_cb_fsize;
+	struct timespec64 ncf_cb_mtime;
+	struct timespec64 ncf_cb_atime;
 
 	unsigned long ncf_cb_flags;
 	bool ncf_file_modified;
diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
--- a/fs/nfsd/xdr4cb.h
+++ b/fs/nfsd/xdr4cb.h
@@ -59,16 +59,20 @@
  * 1: CB_GETATTR opcode (32-bit)
  * N: file_handle
  * 1: number of entry in attribute array (32-bit)
- * 1: entry 0 in attribute array (32-bit)
+ * 3: entry 0-2 in attribute array (32-bit * 3)
  */
 #define NFS4_enc_cb_getattr_sz		(cb_compound_enc_hdr_sz +       \
 					cb_sequence_enc_sz +            \
-					1 + enc_nfs4_fh_sz + 1 + 1)
+					1 + enc_nfs4_fh_sz + 1 + 3)
 /*
  * 4: fattr_bitmap_maxsz
  * 1: attribute array len
  * 2: change attr (64-bit)
  * 2: size (64-bit)
+ * 2: atime.seconds (64-bit)
+ * 1: atime.nanoseconds (32-bit)
+ * 2: mtime.seconds (64-bit)
+ * 1: mtime.nanoseconds (32-bit)
  */
 #define NFS4_dec_cb_getattr_sz		(cb_compound_dec_hdr_sz  +      \
-			cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
+			cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
diff --git a/include/linux/time64.h b/include/linux/time64.h
index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
 	return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
 }
 
+static inline bool timespec64_is_epoch(const struct timespec64 *ts)
+{
+	return ts->tv_sec == 0 && ts->tv_nsec == 0;
+}
+
 /*
  * lhs < rhs:  return <0
  * lhs == rhs: return 0

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (7 preceding siblings ...)
  2024-12-09 21:14 ` [PATCH v5 08/10] nfsd: add support for delegated timestamps Jeff Layton
@ 2024-12-09 21:14 ` Jeff Layton
  2024-12-12 21:06   ` Chuck Lever
  2024-12-09 21:14 ` [PATCH v5 10/10] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION Jeff Layton
  2024-12-09 22:33 ` [PATCH v5 00/10] nfsd: implement the "delstid" draft cel
  10 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:14 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

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.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 10/10] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (8 preceding siblings ...)
  2024-12-09 21:14 ` [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR Jeff Layton
@ 2024-12-09 21:14 ` Jeff Layton
  2024-12-09 22:33 ` [PATCH v5 00/10] nfsd: implement the "delstid" draft cel
  10 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2024-12-09 21:14 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton

Allow clients to request getting a delegation xor an open stateid if a
delegation isn't available. This allows the client to avoid sending a
final CLOSE for the (useless) open stateid, when it is granted a
delegation.

If this flag is requested by the client and there isn't already a new
open stateid, discard the new open stateid before replying.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 24 +++++++++++++++++++++++-
 fs/nfsd/nfs4xdr.c   |  3 ++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a76e35f86021c5657e31e4fddf08cb5781f01e32..82d2aee484d9ee6604a8096227e0a958d58d3128 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6133,6 +6133,17 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 	 */
 }
 
+/* Are we only returning a delegation stateid? */
+static bool open_xor_delegation(struct nfsd4_open *open)
+{
+	if (!(open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
+		return false;
+	/* Did we actually get a delegation? */
+	if (!deleg_is_read(open->op_delegate_type) && !deleg_is_write(open->op_delegate_type))
+		return false;
+	return true;
+}
+
 /**
  * nfsd4_process_open2 - finish open processing
  * @rqstp: the RPC transaction being executed
@@ -6230,6 +6241,17 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	* OPEN succeeds even if we fail.
 	*/
 	nfs4_open_delegation(open, stp, &resp->cstate.current_fh);
+
+	/*
+	 * If there is an existing open stateid, it must be updated and
+	 * returned. Only respect WANT_OPEN_XOR_DELEGATION when a new
+	 * open stateid would have to be created.
+	 */
+	if (new_stp && open_xor_delegation(open)) {
+		memcpy(&open->op_stateid, &zero_stateid, sizeof(open->op_stateid));
+		open->op_rflags |= OPEN4_RESULT_NO_OPEN_STATEID;
+		release_open_stateid(stp);
+	}
 nodeleg:
 	status = nfs_ok;
 	trace_nfsd_open(&stp->st_stid.sc_stateid);
@@ -6246,7 +6268,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	/*
 	* To finish the open response, we just need to set the rflags.
 	*/
-	open->op_rflags = NFS4_OPEN_RESULT_LOCKTYPE_POSIX;
+	open->op_rflags |= NFS4_OPEN_RESULT_LOCKTYPE_POSIX;
 	if (nfsd4_has_session(&resp->cstate))
 		open->op_rflags |= NFS4_OPEN_RESULT_MAY_NOTIFY_LOCK;
 	else if (!(open->op_openowner->oo_flags & NFS4_OO_CONFIRMED))
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index ce93a31ac5cec75b0f944d288e796e7a73641572..2b6a64b4d2a3e3295b9f72e2993dd469945b6114 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3420,7 +3420,8 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 #define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL)		| \
-					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS))
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS)	| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
 
 #define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
 				 BIT(OPEN_ARGS_OPEN_CLAIM_PREVIOUS)	| \

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 00/10] nfsd: implement the "delstid" draft
  2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
                   ` (9 preceding siblings ...)
  2024-12-09 21:14 ` [PATCH v5 10/10] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION Jeff Layton
@ 2024-12-09 22:33 ` cel
  10 siblings, 0 replies; 20+ messages in thread
From: cel @ 2024-12-09 22:33 UTC (permalink / raw)
  To: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker, Jeff Layton
  Cc: Chuck Lever, linux-nfs, linux-kernel, linux-doc

From: Chuck Lever <chuck.lever@oracle.com>

On Mon, 09 Dec 2024 16:13:52 -0500, Jeff Layton wrote:
> We had a report from the kernel test robot that adding support for
> OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION caused an "App Overhead"
> regression in the fs_mark benchmark, and we dropped that series for
> v6.13.
> 
> I've not been able to reproduce this problem. Even on the real hardware
> to which I have access, I don't see the regression in App Overhead
> values that the KTR is reporting in that test.xi
> 
> [...]

Applied to nfsd-testing for v6.14, thanks!

[01/10] nfsd: fix handling of delegated change attr in CB_GETATTR
        commit: be53fa67d813cc134809723699fe96b8dcdf69d3
[02/10] nfs_common: make include/linux/nfs4.h include generated nfs4_1.h
        commit: ba432f5dc998369372737b50d45e9cd5bb221b78
[03/10] nfsd: switch to autogenerated definitions for open_delegation_type4
        commit: 5508d620b0c77c979dcea271ab40e66f1065e55d
[04/10] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
        commit: 1fcab4e8e19b75f77b1f966787272a86f8b1a191
[05/10] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
        commit: 103d2fab19ee8f9cde323b7b2d1b9057451fecb4
[06/10] nfsd: add support for FATTR4_OPEN_ARGUMENTS
        commit: c47967f05d73859bb1f6faeb7eead7fe87f92f3c
[07/10] nfsd: rework NFS4_SHARE_WANT_* flag handling
        commit: f710fdaf971eb5889c6399fdf3dac9fd27604184
[08/10] nfsd: add support for delegated timestamps
        commit: 262ddeaebc5930998f9366b2d91471575d9dff16
[09/10] nfsd: handle delegated timestamps in SETATTR
        commit: 7fbc290538d9b00d34bc0e4aede0b45348a4b957
[10/10] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION
        commit: fca2cd592b6ad1b3809abf7ba27e20d8a7a433c4

--
Chuck Lever


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-09 21:14 ` [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR Jeff Layton
@ 2024-12-12 21:06   ` Chuck Lever
  2024-12-13 14:14     ` Jeff Layton
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2024-12-12 21:06 UTC (permalink / raw)
  To: Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-12 21:06   ` Chuck Lever
@ 2024-12-13 14:14     ` Jeff Layton
  2024-12-13 14:18       ` Chuck Lever
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2024-12-13 14:14 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-13 14:14     ` Jeff Layton
@ 2024-12-13 14:18       ` Chuck Lever
  2024-12-14 14:55         ` Jeff Layton
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2024-12-13 14:18 UTC (permalink / raw)
  To: Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-13 14:18       ` Chuck Lever
@ 2024-12-14 14:55         ` Jeff Layton
  2024-12-14 16:34           ` Chuck Lever
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2024-12-14 14:55 UTC (permalink / raw)
  To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-14 14:55         ` Jeff Layton
@ 2024-12-14 16:34           ` Chuck Lever
  2024-12-14 17:02             ` Chuck Lever
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2024-12-14 16:34 UTC (permalink / raw)
  To: Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-14 16:34           ` Chuck Lever
@ 2024-12-14 17:02             ` Chuck Lever
  2024-12-15 18:52               ` Chuck Lever
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2024-12-14 17:02 UTC (permalink / raw)
  To: Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
  2024-12-14 17:02             ` Chuck Lever
@ 2024-12-15 18:52               ` Chuck Lever
  0 siblings, 0 replies; 20+ messages in thread
From: Chuck Lever @ 2024-12-15 18:52 UTC (permalink / raw)
  To: Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Jonathan Corbet, Trond Myklebust, Anna Schumaker
  Cc: linux-nfs, linux-kernel, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR
  2024-12-09 21:13 ` [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR Jeff Layton
@ 2025-01-19 15:19   ` Chuck Lever
  0 siblings, 0 replies; 20+ messages in thread
From: Chuck Lever @ 2025-01-19 15:19 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-nfs, linux-kernel, linux-doc, Anna Schumaker,
	Trond Myklebust, Jonathan Corbet, Dai Ngo, Tom Talpey,
	Olga Kornievskaia, Neil Brown

On 12/9/24 4:13 PM, Jeff Layton wrote:
> RFC8881, section 10.4.3 has some specific guidance as to how the
> delegated change attribute should be handled. We currently don't follow
> that guidance properly.
> 
> In particular, when the file is modified, the server always reports the
> initial change attribute + 1. Section 10.4.3 however indicates that it
> should be incremented on every GETATTR request from other clients.
> 
> Only request the change attribute until the file has been modified. If
> there is an outstanding delegation, then increment the cached change
> attribute on every GETATTR.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

After review of LTS backport instructions in the candidate patches for
v6.14, this commit stuck out. Should it have a Fixes: or Cc: stable ?

How about

   Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback") ??


> ---
>   fs/nfsd/nfs4callback.c |  8 +++++---
>   fs/nfsd/nfs4xdr.c      | 15 +++++++++------
>   2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 3877b53e429fd89899d7dc35086bee8bda6eed07..25acb8624b854f5d0d184efec660e1f72cad8885 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -361,12 +361,14 @@ static void
>   encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
>   			struct nfs4_cb_fattr *fattr)
>   {
> -	struct nfs4_delegation *dp =
> -		container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> +	struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
>   	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> +	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
>   	u32 bmap[1];
>   
> -	bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE;
> +	bmap[0] = FATTR4_WORD0_SIZE;
> +	if (!ncf->ncf_file_modified)
> +		bmap[0] |= FATTR4_WORD0_CHANGE;
>   
>   	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
>   	encode_nfs_fh4(xdr, fh);
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 53fac037611c05ff8ba2618f9e324a9cb54c3890..c8e8d3f0dff4bb5288186369aad821906e684db7 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2919,6 +2919,7 @@ struct nfsd4_fattr_args {
>   	struct kstat		stat;
>   	struct kstatfs		statfs;
>   	struct nfs4_acl		*acl;
> +	u64			change_attr;
>   #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
>   	void			*context;
>   	int			contextlen;
> @@ -3018,7 +3019,6 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
>   					 const struct nfsd4_fattr_args *args)
>   {
>   	const struct svc_export *exp = args->exp;
> -	u64 c;
>   
>   	if (unlikely(exp->ex_flags & NFSEXP_V4ROOT)) {
>   		u32 flush_time = convert_to_wallclock(exp->cd->flush_time);
> @@ -3029,9 +3029,7 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
>   			return nfserr_resource;
>   		return nfs_ok;
>   	}
> -
> -	c = nfsd4_change_attribute(&args->stat);
> -	return nfsd4_encode_changeid4(xdr, c);
> +	return nfsd4_encode_changeid4(xdr, args->change_attr);
>   }
>   
>   static __be32 nfsd4_encode_fattr4_size(struct xdr_stream *xdr,
> @@ -3556,11 +3554,16 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
>   	if (dp) {
>   		struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
>   
> -		if (ncf->ncf_file_modified)
> +		if (ncf->ncf_file_modified) {
> +			++ncf->ncf_initial_cinfo;
>   			args.stat.size = ncf->ncf_cur_fsize;
> -
> +		}
> +		args.change_attr = ncf->ncf_initial_cinfo;
>   		nfs4_put_stid(&dp->dl_stid);
> +	} else {
> +		args.change_attr = nfsd4_change_attribute(&args.stat);
>   	}
> +
>   	if (err)
>   		goto out_nfserr;
>   
> 


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-01-19 15:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-09 21:13 [PATCH v5 00/10] nfsd: implement the "delstid" draft Jeff Layton
2024-12-09 21:13 ` [PATCH v5 01/10] nfsd: fix handling of delegated change attr in CB_GETATTR Jeff Layton
2025-01-19 15:19   ` Chuck Lever
2024-12-09 21:13 ` [PATCH v5 02/10] nfs_common: make include/linux/nfs4.h include generated nfs4_1.h Jeff Layton
2024-12-09 21:13 ` [PATCH v5 03/10] nfsd: switch to autogenerated definitions for open_delegation_type4 Jeff Layton
2024-12-09 21:13 ` [PATCH v5 04/10] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_* Jeff Layton
2024-12-09 21:13 ` [PATCH v5 05/10] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations Jeff Layton
2024-12-09 21:13 ` [PATCH v5 06/10] nfsd: add support for FATTR4_OPEN_ARGUMENTS Jeff Layton
2024-12-09 21:13 ` [PATCH v5 07/10] nfsd: rework NFS4_SHARE_WANT_* flag handling Jeff Layton
2024-12-09 21:14 ` [PATCH v5 08/10] nfsd: add support for delegated timestamps Jeff Layton
2024-12-09 21:14 ` [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR Jeff Layton
2024-12-12 21:06   ` Chuck Lever
2024-12-13 14:14     ` Jeff Layton
2024-12-13 14:18       ` Chuck Lever
2024-12-14 14:55         ` Jeff Layton
2024-12-14 16:34           ` Chuck Lever
2024-12-14 17:02             ` Chuck Lever
2024-12-15 18:52               ` Chuck Lever
2024-12-09 21:14 ` [PATCH v5 10/10] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION Jeff Layton
2024-12-09 22:33 ` [PATCH v5 00/10] nfsd: implement the "delstid" draft cel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).