Hi Mike, On 8/31/24 6:37 PM, Mike Snitzer wrote: > Hi, > > Happy Labor Day weekend (US holiday on Monday)! Seems apropos to send > what I hope the final LOCALIO patchset this weekend: its my birthday > this coming Tuesday, so _if_ LOCALIO were to get merged for 6.12 > inclusion sometime next week: best b-day gift in a while! ;) > > Anyway, I've been busy incorporating all the review feedback from v14 > _and_ working closely with NeilBrown to address some lingering net-ns > refcounting and nfsd modules refcounting issues, and more (Chnagelog > below): > I've been running tests on localio this afternoon after finishing up going through v15 of the patches (I was most of the way through when you posted v16, so I haven't updated yet!). Cthon tests passed on all NFS versions, and xfstests passed on NFS v4.x. However, I saw this crash from xfstests with NFS v3: [ 1502.440896] run fstests generic/633 at 2024-09-06 14:04:17 [ 1502.694356] process 'vfstest' launched '/dev/fd/4/file1' with NULL argv: empty string added [ 1502.699514] Oops: general protection fault, probably for non-canonical address 0x6c616e69665f6140: 0000 [#1] PREEMPT SMP NOPTI [ 1502.700970] CPU: 3 UID: 0 PID: 513 Comm: nfsd Not tainted 6.11.0-rc6-g0c79a48cd64d-dirty+ #42323 70d41673e6cbf8e3437eb227e0a9c3c46ed3b289 [ 1502.702506] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 2/2/2022 [ 1502.703593] RIP: 0010:nfsd_cache_lookup+0x2b3/0x840 [nfsd] [ 1502.704474] Code: 8d bb 30 02 00 00 bb 01 00 00 00 eb 12 49 8d 46 10 48 8b 08 ff c3 48 85 c9 0f 84 9c 00 00 00 49 89 ce 4c 8d 61 c8 41 8b 45 00 <3b> 41 c8 75 1f 41 8b 45 04 41 3b 46 cc 74 15 8b 15 2c c6 b8 f2 be [ 1502.706931] RSP: 0018:ffffc27ac0a2fd18 EFLAGS: 00010206 [ 1502.707547] RAX: 00000000b95691f7 RBX: 0000000000000002 RCX: 6c616e69665f6178 [ 1502.708311] RDX: 0000000000000034 RSI: ffffa0f8a652a780 RDI: ffffa0f8c04cfb00 [ 1502.709055] RBP: ffffa0f8827b2ba0 R08: 0000000000000000 R09: ffffa0f8c04cfb00 [ 1502.709728] R10: 000000000000009c R11: ffffffffc0c77ef0 R12: 6c616e69665f6140 [ 1502.710382] R13: ffffa0f8c04cfb00 R14: 6c616e69665f6178 R15: ffffa0f883d4e230 [ 1502.710982] FS: 0000000000000000(0000) GS:ffffa0f8fbd80000(0000) knlGS:0000000000000000 [ 1502.711645] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1502.712087] CR2: 00007f2c4d1ed640 CR3: 0000000117a1e000 CR4: 0000000000750ef0 [ 1502.712615] PKRU: 55555554 [ 1502.712804] Call Trace: [ 1502.712979] <TASK> [ 1502.713131] ? __die_body+0x6a/0xb0 [ 1502.713372] ? die_addr+0xa4/0xd0 [ 1502.713583] ? exc_general_protection+0x16c/0x210 [ 1502.713880] ? asm_exc_general_protection+0x26/0x30 [ 1502.714164] ? __pfx_nfs3svc_decode_sattrargs+0x10/0x10 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.714700] ? nfsd_cache_lookup+0x2b3/0x840 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.715156] ? nfsd_cache_lookup+0x2e7/0x840 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.715590] nfsd_dispatch+0x93/0x210 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.715997] svc_process_common+0x324/0x680 [sunrpc 2f7328527f188558dea7880294960ba75bb09c81] [ 1502.716439] ? __pfx_nfsd_dispatch+0x10/0x10 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.716873] svc_process+0x117/0x1c0 [sunrpc 2f7328527f188558dea7880294960ba75bb09c81] [ 1502.717276] svc_recv+0xabf/0xc00 [sunrpc 2f7328527f188558dea7880294960ba75bb09c81] [ 1502.717674] nfsd+0xc5/0x100 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.718225] ? __pfx_nfsd+0x10/0x10 [nfsd a9c12e0cc9647b021c55f7745e60fc1cbe54674a] [ 1502.718641] kthread+0xe9/0x110 [ 1502.718798] ? __pfx_kthread+0x10/0x10 [ 1502.718979] ret_from_fork+0x37/0x50 [ 1502.719154] ? __pfx_kthread+0x10/0x10 [ 1502.719335] ret_from_fork_asm+0x1a/0x30 [ 1502.719525] </TASK> [ 1502.719636] Modules linked in: nfsv3 overlay cbc cts rpcsec_gss_krb5 nfsv4 nfs rpcrdma rdma_cm iw_cm ib_cm cfg80211 ib_core rfkill 8021q garp stp mrp llc vfat fat intel_rapl_msr intel_rapl_common intel_uncore_frequency_common intel_pmc_core intel_vsec pmt_telemetry pmt_class kvm_intel kvm snd_hda_codec_generic snd_hda_intel snd_intel_dspcfg crct10dif_pclmul crc32_pclmul snd_hda_codec polyval_clmulni polyval_generic ghash_clmulni_intel snd_hwdep sha512_ssse3 snd_hda_core sha256_ssse3 sha1_ssse3 iTCO_wdt snd_pcm intel_pmc_bxt iTCO_vendor_support aesni_intel snd_timer gf128mul snd psmouse crypto_simd i2c_i801 cryptd joydev pcspkr rapl lpc_ich i2c_smbus soundcore mousedev mac_hid nfsd nfs_acl lockd auth_rpcgss grace nfs_localio sunrpc usbip_host dm_mod usbip_core loop nfnetlink vsock_loopback vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vmw_vmci vsock qemu_fw_cfg ip_tables x_tables hid_generic usbhid xfs libcrc32c crc32c_generic serio_raw atkbd libps2 virtio_net vivaldi_fmap virtio_gpu virtio_console [ 1502.719684] net_failover virtio_blk crc32c_intel i8042 failover virtio_rng xhci_pci intel_agp virtio_balloon xhci_pci_renesas virtio_dma_buf serio intel_gtt [ 1502.724436] ---[ end trace 0000000000000000 ]--- Please let me know if there are any other details you need about my setup to help debug this! Thanks, Anna > git diff snitzer/nfs-localio-for-next.v14 snitzer/nfs-localio-for-next.v15 | diffstat > Documentation/filesystems/nfs/localio.rst | 106 +++++++++-- > fs/Kconfig | 26 ++ > fs/nfs/Kconfig | 16 - > fs/nfs/client.c | 4 > fs/nfs/flexfilelayout/flexfilelayout.c | 8 > fs/nfs/internal.h | 24 +- > fs/nfs/localio.c | 92 +++------ > fs/nfs/pagelist.c | 4 > fs/nfs/write.c | 4 > fs/nfs_common/nfslocalio.c | 287 +++++++++++------------------- > fs/nfsd/Kconfig | 16 - > fs/nfsd/Makefile | 2 > fs/nfsd/filecache.c | 27 +- > fs/nfsd/filecache.h | 1 > fs/nfsd/localio.c | 79 ++++---- > fs/nfsd/netns.h | 4 > fs/nfsd/nfsctl.c | 25 ++ > fs/nfsd/nfsd.h | 2 > fs/nfsd/nfsfh.c | 3 > fs/nfsd/nfssvc.c | 11 - > fs/nfsd/vfs.h | 5 > include/linux/nfs.h | 2 > include/linux/nfs_fs_sb.h | 3 > include/linux/nfslocalio.h | 64 +++--- > 24 files changed, 410 insertions(+), 405 deletions(-) > > 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 > > Chuck and Jeff, 2 patches have respective Not-Acked-by and > Not-Reviewed-by as placeholders because there were enough changes in > v15 that you'll need to revalidate your provided tags: > [PATCH v15 16/26] nfsd: add LOCALIO support > [PATCH v15 17/26] nfsd: implement server support for NFS_LOCALIO_PROGRAM > > Otherwise, I did add the tags you provided from your review of v14. > Hopefully I didn't miss any. > > Changes since v14 (Thursday): > > - Reviewed, tested, fixed and incorporated NeilBrown's really nice > solution for addressing net-ns refcounting issues he identified > (first I didn't have adequate protection on net-ns then I had too > heavy), see Neil's 6 replacement patches: > https://marc.info/?l=linux-nfs&m=172498546024767&w=2 > > - Reviewed, tested and incorporated NeilBrown's __module_get > improvements that build on his net-ns changes, see: > https://marc.info/?l=linux-nfs&m=172499598828454&w=2 > > - Added NeilBrown to the Copyright headers of 4 LOCALIO source files, > warranted thanks to his contributions. > > - Switched back from using 'struct nfs_localio_ctx' to 'struct > nfsd_file' thanks to NeilBrown's suggestion, much cleaner: > https://marc.info/?l=linux-nfs&m=172499732628938&w=2 > - added nfsd_file_put_local() to achieve this. > > - Cleaned up and refactored nfsd_open_local_fh(). > > - Removed the more elaborate symbol_request()+symbol_put() code from > nfs_common/nfslocalio.c in favor of having init_nfsd() copy its > nfsd_localio_operations table to 'nfs_to'. > > - Fixed the Kconfig to only need a single CONFIG_NFS_LOCALIO (which > still selects NFS_COMMON_LOCALIO_SUPPORT to control how to build > nfs_common's nfs_local enablement, support nfs_localio.ko). > > - Verified all commits are bisect-clean both with and without > CONFIG_NFS_LOCALIO set. > - required adding some missing #if IS_ENABLED(CONFIG_NFS_LOCALIO) > > - Added various Reviewed-by and Acked-by tags from Chuck and Jeff. > But again, left Not-<tag> placeholders in nfsd patches 16 and 17. > > - Reviwed and updated all patch headers as needed to reflect the above > changes. > > - Updated localio.rst to reflect all changes above and improved > readability after another pass of proofreading. > > - Added FAQ 8 to localio.rst (Chuck's question and Neil's answer about > export options and LOCALIO. > > - Moved verbose patch header content about the 2 major interlocking > strategies used in LOCALIO to a new "NFS Client and Server > Interlock" section in localio.rst (tied it to a new FAQ 9). > > 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 (12): > 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: prepare for the NFS client to use nfsd_file for LOCALIO > nfsd: implement server support for NFS_LOCALIO_PROGRAM > nfs: pass struct nfsd_file to nfs_init_pgio and nfs_init_commit > nfs: implement client support for NFS_LOCALIO_PROGRAM > nfs: add Documentation/filesystems/nfs/localio.rst > nfs: add "NFS Client and Server Interlock" section to 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 | 357 ++++++++++ > fs/Kconfig | 23 + > fs/nfs/Kconfig | 1 + > 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 | 757 ++++++++++++++++++++++ > 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 | 162 +++++ > fs/nfsd/Kconfig | 1 + > fs/nfsd/Makefile | 1 + > fs/nfsd/export.c | 30 +- > fs/nfsd/filecache.c | 103 ++- > fs/nfsd/filecache.h | 5 + > fs/nfsd/localio.c | 189 ++++++ > fs/nfsd/netns.h | 12 +- > fs/nfsd/nfsctl.c | 27 +- > fs/nfsd/nfsd.h | 6 +- > fs/nfsd/nfsfh.c | 137 ++-- > fs/nfsd/nfsfh.h | 2 + > fs/nfsd/nfssvc.c | 102 ++- > fs/nfsd/trace.h | 21 +- > fs/nfsd/vfs.h | 2 + > include/linux/nfs.h | 9 + > include/linux/nfs_common.h | 17 + > include/linux/nfs_fs_sb.h | 9 + > include/linux/nfs_xdr.h | 20 +- > include/linux/nfslocalio.h | 79 +++ > 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 +- > 47 files changed, 2468 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 >