[PATCH v5 00/10] nfsd: implement the "delstid" draft

Jeff Layton posted 10 patches 2 weeks, 2 days ago
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(-)
[PATCH v5 00/10] nfsd: implement the "delstid" draft
Posted by Jeff Layton 2 weeks, 2 days ago
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>
Re: [PATCH v5 00/10] nfsd: implement the "delstid" draft
Posted by cel@kernel.org 2 weeks, 2 days ago
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