Message ID | 165996657035.2637.4745479232455341597.stgit@manet.1015granger.net (mailing list archive) |
---|---|
Headers | show |
Series | Wait for DELEGRETURN before returning NFS4ERR_DELAY | expand |
On Mon, 2022-08-08 at 09:52 -0400, Chuck Lever wrote: > This RFC series adds logic to the Linux NFS server to make it wait a > few moments for clients to return a delegation before replying with > NFS4ERR_DELAY. There are two main benefits: > > - This improves responsiveness when a delegated file is accessed > from another client > - This removes an unnecessary latency if a client has neglected to > return a delegation before attempting a RENAME or SETATTR > > This series is incomplete: > - There are likely other operations that can benefit (eg. OPEN) > > This series applies against v5.19. > > Changes since v2: > - Wake immediately after client sends DELEGRETURN > - More tracepoint improvements > > Changes since RFC: > - Eliminate spurious console messages on the server > - Wait for DELEGRETURN for RENAME operations ^^^^ Does REMOVE need similar treatment? Does the Apple client return a delegation before attempting to unlink? > - Add CB done tracepoints > - Rework the retry loop > > --- > > Chuck Lever (7): > NFSD: Instrument fh_verify() > NFSD: Replace dprintk() call site in fh_verify() > NFSD: Trace NFSv4 COMPOUND tags > NFSD: Add tracepoints to report NFSv4 callback completions > NFSD: Add a mechanism to wait for a DELEGRETURN > NFSD: Make nfsd4_setattr() wait before returning NFS4ERR_DELAY > NFSD: Make nfsd4_rename() wait before returning NFS4ERR_DELAY > > > fs/nfsd/nfs4layouts.c | 2 +- > fs/nfsd/nfs4proc.c | 56 +++++++++++--- > fs/nfsd/nfs4state.c | 35 +++++++++ > fs/nfsd/nfsd.h | 1 + > fs/nfsd/nfsfh.c | 13 +--- > fs/nfsd/trace.h | 171 ++++++++++++++++++++++++++++++++++++++++-- > fs/nfsd/xdr4.h | 2 + > 7 files changed, 251 insertions(+), 29 deletions(-) > > -- > Chuck Lever > The new tracepoints are nice and the patchset makes sense. You can add: Reviewed-by: Jeff Layton <jlayton@kernel.org>
> On Aug 8, 2022, at 2:48 PM, Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2022-08-08 at 09:52 -0400, Chuck Lever wrote: >> This RFC series adds logic to the Linux NFS server to make it wait a >> few moments for clients to return a delegation before replying with >> NFS4ERR_DELAY. There are two main benefits: >> >> - This improves responsiveness when a delegated file is accessed >> from another client >> - This removes an unnecessary latency if a client has neglected to >> return a delegation before attempting a RENAME or SETATTR >> >> This series is incomplete: >> - There are likely other operations that can benefit (eg. OPEN) > Does REMOVE need similar treatment? Does the Apple client return a > delegation before attempting to unlink? It's certainly plausible that REMOVE might trigger a delegation recall, but I haven't yet seen this happen on the wire. -- Chuck Lever
> On Aug 8, 2022, at 4:06 PM, Chuck Lever III <chuck.lever@oracle.com> wrote: > >> On Aug 8, 2022, at 2:48 PM, Jeff Layton <jlayton@kernel.org> wrote: >> >> On Mon, 2022-08-08 at 09:52 -0400, Chuck Lever wrote: >>> This RFC series adds logic to the Linux NFS server to make it wait a >>> few moments for clients to return a delegation before replying with >>> NFS4ERR_DELAY. There are two main benefits: >>> >>> - This improves responsiveness when a delegated file is accessed >>> from another client >>> - This removes an unnecessary latency if a client has neglected to >>> return a delegation before attempting a RENAME or SETATTR >>> >>> This series is incomplete: >>> - There are likely other operations that can benefit (eg. OPEN) > >> Does REMOVE need similar treatment? Does the Apple client return a >> delegation before attempting to unlink? > > It's certainly plausible that REMOVE might trigger a delegation recall, > but I haven't yet seen this happen on the wire. I started playing with REMOVE and DELEG15a, and ran into an interesting problem. REMOVE is passed the filehandle of the parent and the name of the entry to be removed. nfsd4_remove() doesn't have an inode or filehandle of the object to be removed. nfsd_unlink() does acquire the inode of that object, though. And, I guess if we want to avoid NFSv3 operations returning JUKEBOX on delegated files too, the "wait for DELEGRETURN" really ought to be called from the fs/nfsd/vfs.c functions, not from the NFSv4 specific functions in fs/nfsd/nfs4proc.c. -- Chuck Lever