| Message ID | 20241209-delstid-v5-0-42308228f692@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | nfsd: implement the "delstid" draft | expand |
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
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,