| Message ID | 20240828004445.22634-1-cel@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Split up refactoring of fh_verify() | expand |
On Tue, Aug 27, 2024 at 08:44:39PM -0400, cel@kernel.org wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > These six patches are meant to replace: > > nfsd: factor out __fh_verify to allow NULL rqstp to be passed > nfsd: add nfsd_file_acquire_local() > > I've removed the @nfs_vers parameter to __fh_verify(), and tried > something a little different with the trace points. Please check > my math. > > These have been compile-tested, but no more than that. Interested in > comments. Reviewed quickly just now, nicely done! I'll go over it very carefully now by replacing the 2 patches you noted and updating the localio patches thaa follow. I noticed some typos and mentioning nfs_vers usage in a header despite you having removed the need to pass it. So I'll fix up those nits along the way. But I just wanted to immediately say: thanks! > Chuck Lever (2): > NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() > NFSD: Short-circuit fh_verify tracepoints for LOCALIO > > NeilBrown (4): > 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() > > fs/nfsd/export.c | 29 ++++-- > fs/nfsd/filecache.c | 61 +++++++++++-- > fs/nfsd/filecache.h | 3 + > fs/nfsd/lockd.c | 6 +- > fs/nfsd/nfsfh.c | 210 ++++++++++++++++++++++++++------------------ > fs/nfsd/nfsfh.h | 5 ++ > fs/nfsd/trace.h | 18 ++-- > 7 files changed, 223 insertions(+), 109 deletions(-) > > -- > 2.45.2 >
On Tue, Aug 27, 2024 at 09:08:32PM -0400, Mike Snitzer wrote: > On Tue, Aug 27, 2024 at 08:44:39PM -0400, cel@kernel.org wrote: > > From: Chuck Lever <chuck.lever@oracle.com> > > > > These six patches are meant to replace: > > > > nfsd: factor out __fh_verify to allow NULL rqstp to be passed > > nfsd: add nfsd_file_acquire_local() > > > > I've removed the @nfs_vers parameter to __fh_verify(), and tried > > something a little different with the trace points. Please check > > my math. > > > > These have been compile-tested, but no more than that. Interested in > > comments. > > Reviewed quickly just now, nicely done! > > I'll go over it very carefully now by replacing the 2 patches you > noted and updating the localio patches thaa follow. I noticed some > typos and mentioning nfs_vers usage in a header despite you having > removed the need to pass it. So I'll fix up those nits along the way. > > But I just wanted to immediately say: thanks! I reviewed and incorporated your 6 patches and successfully tested the result (by issuing IO with and without LOCALIO enabled with both NFS v3 and NFS v4.2). I've pushed the latest code to my branch: https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next I'm approaching the end of addressing all the v13 review feedback, but still have to sort through a couple issues that Neil pointed out about nfs_local_disable() racing with nfsd_open_local_fh(). But in case you're curious here is a preview of the changelog for what'll be in v14 (hopefully will post by end of day tomorrow): - 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. - Update 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. - 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. - Improved "Always allow LOCALIO" comment in check_nfsd_access() and added Kdoc above check_nfsd_access(), 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.
From: Chuck Lever <chuck.lever@oracle.com> These six patches are meant to replace: nfsd: factor out __fh_verify to allow NULL rqstp to be passed nfsd: add nfsd_file_acquire_local() I've removed the @nfs_vers parameter to __fh_verify(), and tried something a little different with the trace points. Please check my math. These have been compile-tested, but no more than that. Interested in comments. Chuck Lever (2): NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() NFSD: Short-circuit fh_verify tracepoints for LOCALIO NeilBrown (4): 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() fs/nfsd/export.c | 29 ++++-- fs/nfsd/filecache.c | 61 +++++++++++-- fs/nfsd/filecache.h | 3 + fs/nfsd/lockd.c | 6 +- fs/nfsd/nfsfh.c | 210 ++++++++++++++++++++++++++------------------ fs/nfsd/nfsfh.h | 5 ++ fs/nfsd/trace.h | 18 ++-- 7 files changed, 223 insertions(+), 109 deletions(-)