Re: [PATCH v15 00/26] nfs/nfsd: add support for LOCALIO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux