On Tue, Aug 27, 2024 at 09:08:32PM -0400, Mike Snitzer wrote: > On Tue, Aug 27, 2024 at 08:44:39PM -0400, cel@xxxxxxxxxx wrote: > > From: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > 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.
