| Message ID | 20240829010424.83693-1-snitzer@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | nfs/nfsd: add support for LOCALIO | expand |
On Wed, Aug 28, 2024 at 09:03:55PM -0400, Mike Snitzer wrote: > These latest changes are available in my git tree here: > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next > > I _think_ I addressed all of v13's very helpful review comments. > Special thanks to Neil and Chuck for their time and help! > > And hopefully I didn't miss anything in the changelog below. As it happens, a last minute rebase that I did just before sending out v14 caused me to send out 2 stale patches: [PATCH v14 09/25] nfsd: add nfsd_file_acquire_local() [PATCH v14 25/25] nfs: add FAQ section to Documentation/filesystems/nfs/localio.rst I will reply to each patch with a correct v14.5 for each. Sorry for the confusion. Here is the incremental diff that shows what was missing in v14: diff --git a/Documentation/filesystems/nfs/localio.rst b/Documentation/filesystems/nfs/localio.rst index 4b6d63246479..5d652f637a97 100644 --- a/Documentation/filesystems/nfs/localio.rst +++ b/Documentation/filesystems/nfs/localio.rst @@ -120,12 +120,13 @@ FAQ using RPC, beneficial? Is the benefit pNFS specific? Avoiding the use of XDR and RPC for file opens is beneficial to - performance regardless of whether pNFS is used. However adding a - requirement to go over the wire to do an open and/or close ends up - negating any benefit of avoiding the wire for doing the I/O itself - when we´re dealing with small files. There is no benefit to replacing - the READ or WRITE with a new open and/or close operation that still - needs to go over the wire. + performance regardless of whether pNFS is used. Especially when + dealing with small files its best to avoid going over the wire + whenever possible, otherwise it could reduce or even negate the + benefits of avoiding the wire for doing the small file I/O itself. + Given LOCALIO's requirements the current approach of having the + client perform a server-side file open, without using RPC, is ideal. + If in the future requirements change then we can adapt accordingly. 7. Why is LOCALIO only supported with UNIX Authentication (AUTH_UNIX)? diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c index e636d2a1e664..46a7f9b813e5 100644 --- a/fs/nfsd/lockd.c +++ b/fs/nfsd/lockd.c @@ -32,10 +32,8 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp, int access; struct svc_fh fh; - if (rqstp->rq_vers == 4) - fh_init(&fh, NFS3_FHSIZE); - else - fh_init(&fh, NFS_FHSIZE); + /* must initialize before using! but maxsize doesn't matter */ + fh_init(&fh,0); fh.fh_handle.fh_size = f->size; memcpy(&fh.fh_handle.fh_raw, f->data, f->size); fh.fh_export = NULL; diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c index 49468e478d23..eca577cf3263 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -290,9 +290,6 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net, fhp->fh_use_wgather = true; if (exp->ex_flags & NFSEXP_V4ROOT) goto out; - break; - case 0: - WARN_ONCE(1, "Uninitialized file handle"); } return 0;
On Wed, Aug 28, 2024 at 09:42:53PM -0400, Mike Snitzer wrote: > On Wed, Aug 28, 2024 at 09:03:55PM -0400, Mike Snitzer wrote: > > These latest changes are available in my git tree here: > > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next > > > > I _think_ I addressed all of v13's very helpful review comments. > > Special thanks to Neil and Chuck for their time and help! > > > > And hopefully I didn't miss anything in the changelog below. > > As it happens, a last minute rebase that I did just before sending out > v14 caused me to send out 2 stale patches: I meant these were stale: [PATCH v14 06/25] NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() [PATCH v14 25/25] nfs: add FAQ section to Documentation/filesystems/nfs/localio.rst But I've now sent v14.5 to fix each... > Sorry for the confusion. Again ;)
These latest changes are available in my git tree here: https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next I _think_ I addressed all of v13's very helpful review comments. Special thanks to Neil and Chuck for their time and help! And hopefully I didn't miss anything in the changelog below. Changes since v13: - Extended the nn->nfsd_serv reference lifetime to be identical to the nfsd_file (until after localio's IO is complete), suggested by Neil. This is made easier by introducing a 'struct nfs_localio_ctx' that contains both the 'nfsd_file' and 'nfsd_net' associated with localio. - Switched nfs_common's 'nfs_to' symbol management locking from using mutex to spinlock, suggested by Neil. - Eliminated nfs_local_file_open() by folding it into nfs_local_open_fh(), suggested by Neil. - Updated nfs_uuid_is_local() to get reference on the net, drop it in nfs_local_disable(), suggested by Neil. - Pushed saving/restoring of client's cred down from nfsd_open_local_fh() to nfsd_file_acquire_local(), suggested by Neil. - Dropped the pNFS flexfiles-specific open file caching that caused lifetime issues (inability to unmount backing filesystem), noticed by Neil. Also removed nfsd_file dummy definition as a side-effect. - Updated NFSD_LOCALIO in fs/nfsd/Kconfig to explicitly 'default n' and improve description, suggested by Chuck. Also made the same updates to NFS_LOCALIO in fs/nfs/Kconfig. - Split out a separate preliminary patch that introduces nfsd_serv_try_get() and nfsd_serv_put() and the associated percpu_ref, suggested by Chuck. - Moved rpcauth_map_clnt_to_svc_cred_local from net/sunrpc/auth.c to net/sunrpc/svcauth.c and renamed it to svcauth_map_clnt_to_svc_cred_local. Also added kdoc. Suggested by Chuck. - Added Chuck's Acked-by to 2 patches. - Incorporated Chuck's 6 patches that split up and improved the __fh_verify and nfsd_file_acquire_local patches. Added fh_verify_local as Chuck suggested. Used Neil's improved comment for localio's early return from check_nfsd_access. - Revised the answer to FAQ 6 in localio.rst, hopefully for the better. - Fixed issue Neil pointed out about nfs_local_disable() racing with nfsd_open_local_fh() by adding the use of a clp->cl_localio_lock (spinlock_t) and RCU to dereference clp->cl_nfssvc_net and clp->cl_nfssvc_dom. The call to nfsd_open_local_fh() is covered by RCU. - Split the patch "nfs_common: add NFS LOCALIO auxiliary protocol enablement" out to 3 separate patches. Hope is that it helps reduce review burden thanks to each patch header explaining things with more precision and detail. All review appreciated, thanks! Mike Chuck Lever (2): NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() NFSD: Short-circuit fh_verify tracepoints for LOCALIO Mike Snitzer (11): nfs_common: factor out nfs_errtbl and nfs_stat_to_errno nfs_common: factor out nfs4_errtbl and nfs4_stat_to_errno nfs: factor out {encode,decode}_opaque_fixed to nfs_xdr.h nfsd: add nfsd_serv_try_get and nfsd_serv_put SUNRPC: remove call_allocate() BUG_ONs nfs_common: add NFS LOCALIO auxiliary protocol enablement nfs_common: introduce nfs_localio_ctx struct and interfaces nfsd: implement server support for NFS_LOCALIO_PROGRAM nfs: pass struct nfs_localio_ctx to nfs_init_pgio and nfs_init_commit nfs: implement client support for NFS_LOCALIO_PROGRAM nfs: add Documentation/filesystems/nfs/localio.rst NeilBrown (5): NFSD: Handle @rqstp == NULL in check_nfsd_access() NFSD: Refactor nfsd_setuser_and_check_port() nfsd: factor out __fh_verify to allow NULL rqstp to be passed nfsd: add nfsd_file_acquire_local() SUNRPC: replace program list with program array Trond Myklebust (4): nfs: enable localio for non-pNFS IO pnfs/flexfiles: enable localio support nfs/localio: use dedicated workqueues for filesystem read and write nfs: add FAQ section to Documentation/filesystems/nfs/localio.rst Weston Andros Adamson (3): SUNRPC: add svcauth_map_clnt_to_svc_cred_local nfsd: add localio support nfs: add localio support Documentation/filesystems/nfs/localio.rst | 276 ++++++++ fs/Kconfig | 3 + fs/nfs/Kconfig | 17 + fs/nfs/Makefile | 1 + fs/nfs/client.c | 15 +- fs/nfs/filelayout/filelayout.c | 6 +- fs/nfs/flexfilelayout/flexfilelayout.c | 56 +- fs/nfs/flexfilelayout/flexfilelayoutdev.c | 6 + fs/nfs/inode.c | 57 +- fs/nfs/internal.h | 53 +- fs/nfs/localio.c | 789 ++++++++++++++++++++++ fs/nfs/nfs2xdr.c | 70 +- fs/nfs/nfs3xdr.c | 108 +-- fs/nfs/nfs4xdr.c | 84 +-- fs/nfs/nfstrace.h | 61 ++ fs/nfs/pagelist.c | 16 +- fs/nfs/pnfs_nfs.c | 2 +- fs/nfs/write.c | 12 +- fs/nfs_common/Makefile | 5 + fs/nfs_common/common.c | 134 ++++ fs/nfs_common/nfslocalio.c | 233 +++++++ fs/nfsd/Kconfig | 17 + fs/nfsd/Makefile | 1 + fs/nfsd/export.c | 30 +- fs/nfsd/filecache.c | 98 ++- fs/nfsd/filecache.h | 4 + fs/nfsd/localio.c | 180 +++++ fs/nfsd/lockd.c | 6 +- fs/nfsd/netns.h | 8 +- fs/nfsd/nfsctl.c | 2 +- fs/nfsd/nfsd.h | 6 +- fs/nfsd/nfsfh.c | 141 ++-- fs/nfsd/nfsfh.h | 2 + fs/nfsd/nfssvc.c | 105 ++- fs/nfsd/trace.h | 21 +- fs/nfsd/vfs.h | 7 + include/linux/nfs.h | 9 + include/linux/nfs_common.h | 17 + include/linux/nfs_fs_sb.h | 10 + include/linux/nfs_xdr.h | 20 +- include/linux/nfslocalio.h | 69 ++ include/linux/sunrpc/svc.h | 7 +- include/linux/sunrpc/svcauth.h | 5 + net/sunrpc/clnt.c | 6 - net/sunrpc/svc.c | 68 +- net/sunrpc/svc_xprt.c | 2 +- net/sunrpc/svcauth.c | 28 + net/sunrpc/svcauth_unix.c | 3 +- 48 files changed, 2467 insertions(+), 409 deletions(-) create mode 100644 Documentation/filesystems/nfs/localio.rst create mode 100644 fs/nfs/localio.c create mode 100644 fs/nfs_common/common.c create mode 100644 fs/nfs_common/nfslocalio.c create mode 100644 fs/nfsd/localio.c create mode 100644 include/linux/nfs_common.h create mode 100644 include/linux/nfslocalio.h