mbox series

[v9,00/19] nfs/nfsd: add support for localio

Message ID 20240628211105.54736-1-snitzer@kernel.org (mailing list archive)
Headers show
Series nfs/nfsd: add support for localio | expand

Message

Mike Snitzer June 28, 2024, 9:10 p.m. UTC
Hi,

I'd prefer to see these changes land upstream for 6.11 if possible.
They are adequately Kconfig'd to certainly pose no risk if disabled.
And even if localio enabled it has proven to work well with increased
testing.

Worked with Kent Overstreet to enable testing integration with ktest
running xfstests, the dashboard is here:
https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
(it is running way more xfstests tests than is usual for nfs, would be
good to reconcile that with the listing provided here:
https://wiki.linux-nfs.org/wiki/index.php/Xfstests )

Changes since v8:
- Fixed xfstests generic/355 (clear suid after write) as a side-effect
  of dropping the "nfs/localio: use dedicated workqueues for
  filesystem read and write" patch (XFS is looking at the security
  context of the task... which is really odd!)
- Refactored and fixed nfs_local_vfs_getattr() to support NFS v4 as
  requested by Neil.
- Fixed potential for localio file opens to prevent nfsd from shutting
  down (as caught by Jeff's helpful review) by switching to using
  percpu_ref_tryget_live (and renamed nfsd_serv_get to
  nfsd_serv_try_get).
- Removed all dprintk() from fs/nfsd/localio.c
- Removed one dprintk() from fs/nfs/localio.c, left others because the
  nfs client maintainers don't seem so against them (they do require
  explicit enablement after all).

TODO:
- Hopefully get a favorable response to this patch from XFS engineers:
  https://marc.info/?l=linux-xfs&m=171959152810706&w=2
  (otherwise, will need to revisit using dedicated workqueue patch)

All review and comments are welcome!

Thanks,
Mike

My git tree is here:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/

This v9 is both branch nfs-localio-for-6.11 (always tracks latest)
and nfs-localio-for-6.11.v9

Mike Snitzer (11):
  nfs_common: add NFS LOCALIO auxiliary protocol enablement
  nfs/localio: fix nfs_localio_vfs_getattr() to properly support v4
  nfs: factor out {encode,decode}_opaque_fixed to nfs_xdr.h
  SUNRPC: remove call_allocate() BUG_ON if p_arglen=0 to allow RPC with void arg
  nfs: implement client support for NFS_LOCALIO_PROGRAM
  nfsd: add "localio" support
  nfsd/localio: manage netns reference in nfsd_open_local_fh
  nfsd: use percpu_ref to interlock nfsd_destroy_serv and nfsd_open_local_fh
  nfsd: add Kconfig options to allow localio to be enabled
  nfsd: implement server support for NFS_LOCALIO_PROGRAM
  nfs: add Documentation/filesystems/nfs/localio.rst

NeilBrown (1):
  SUNRPC: replace program list with program array

Trond Myklebust (2):
  nfs: enable localio for non-pNFS I/O
  pnfs/flexfiles: Enable localio for flexfiles I/O

Weston Andros Adamson (5):
  nfs: pass nfs_client to nfs_initiate_pgio
  nfs: pass descriptor thru nfs_initiate_pgio path
  nfs: pass struct file to nfs_init_pgio and nfs_init_commit
  sunrpc: add rpcauth_map_to_svc_cred_local
  nfs: add "localio" support

 Documentation/filesystems/nfs/localio.rst | 135 ++++
 fs/Kconfig                                |   3 +
 fs/nfs/Kconfig                            |  14 +
 fs/nfs/Makefile                           |   1 +
 fs/nfs/blocklayout/blocklayout.c          |   6 +-
 fs/nfs/client.c                           |  15 +-
 fs/nfs/filelayout/filelayout.c            |  16 +-
 fs/nfs/flexfilelayout/flexfilelayout.c    | 131 +++-
 fs/nfs/flexfilelayout/flexfilelayout.h    |   2 +
 fs/nfs/flexfilelayout/flexfilelayoutdev.c |   6 +
 fs/nfs/inode.c                            |   4 +
 fs/nfs/internal.h                         |  60 +-
 fs/nfs/localio.c                          | 827 ++++++++++++++++++++++
 fs/nfs/nfs4xdr.c                          |  13 -
 fs/nfs/nfstrace.h                         |  61 ++
 fs/nfs/pagelist.c                         |  32 +-
 fs/nfs/pnfs.c                             |  24 +-
 fs/nfs/pnfs.h                             |   6 +-
 fs/nfs/pnfs_nfs.c                         |   2 +-
 fs/nfs/write.c                            |  13 +-
 fs/nfs_common/Makefile                    |   3 +
 fs/nfs_common/nfslocalio.c                |  74 ++
 fs/nfsd/Kconfig                           |  14 +
 fs/nfsd/Makefile                          |   1 +
 fs/nfsd/filecache.c                       |   2 +-
 fs/nfsd/localio.c                         | 319 +++++++++
 fs/nfsd/netns.h                           |  12 +-
 fs/nfsd/nfsctl.c                          |   2 +-
 fs/nfsd/nfsd.h                            |   2 +-
 fs/nfsd/nfssvc.c                          | 116 ++-
 fs/nfsd/trace.h                           |   3 +-
 fs/nfsd/vfs.h                             |   9 +
 include/linux/nfs.h                       |   9 +
 include/linux/nfs_fs.h                    |   2 +
 include/linux/nfs_fs_sb.h                 |  10 +
 include/linux/nfs_xdr.h                   |  20 +-
 include/linux/nfslocalio.h                |  41 ++
 include/linux/sunrpc/auth.h               |   4 +
 include/linux/sunrpc/svc.h                |   7 +-
 net/sunrpc/auth.c                         |  15 +
 net/sunrpc/clnt.c                         |   1 -
 net/sunrpc/svc.c                          |  68 +-
 net/sunrpc/svc_xprt.c                     |   2 +-
 net/sunrpc/svcauth_unix.c                 |   3 +-
 44 files changed, 1975 insertions(+), 135 deletions(-)
 create mode 100644 Documentation/filesystems/nfs/localio.rst
 create mode 100644 fs/nfs/localio.c
 create mode 100644 fs/nfs_common/nfslocalio.c
 create mode 100644 fs/nfsd/localio.c
 create mode 100644 include/linux/nfslocalio.h

Comments

Chuck Lever III June 29, 2024, 3:36 p.m. UTC | #1
> On Jun 28, 2024, at 5:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
> 
> Hi,
> 
> I'd prefer to see these changes land upstream for 6.11 if possible.
> They are adequately Kconfig'd to certainly pose no risk if disabled.
> And even if localio enabled it has proven to work well with increased
> testing.

Can v10 split this series into an NFS client part and an NFS
server part? I will need to get the NFSD changes into nfsd-next
in the next week or so to land in v6.11.


> Worked with Kent Overstreet to enable testing integration with ktest
> running xfstests, the dashboard is here:
> https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
> (it is running way more xfstests tests than is usual for nfs, would be
> good to reconcile that with the listing provided here:
> https://wiki.linux-nfs.org/wiki/index.php/Xfstests )

Actually, we're using kdevops for NFSD CI testing. Any possibility
that we can get some help setting that up? (It runs xfstests and
several other workflows).


--
Chuck Lever
Mike Snitzer June 29, 2024, 4:03 p.m. UTC | #2
On Sat, Jun 29, 2024 at 03:36:10PM +0000, Chuck Lever III wrote:
> 
> 
> > On Jun 28, 2024, at 5:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
> > 
> > Hi,
> > 
> > I'd prefer to see these changes land upstream for 6.11 if possible.
> > They are adequately Kconfig'd to certainly pose no risk if disabled.
> > And even if localio enabled it has proven to work well with increased
> > testing.
> 
> Can v10 split this series into an NFS client part and an NFS
> server part? I will need to get the NFSD changes into nfsd-next
> in the next week or so to land in v6.11.

I forgot to mention this as a v9 improvement: I did split the series,
but left it as one patchset.

Patches 1-12 are NFS client, Patches 13-19 are NFSD.

Here is the diffstat for NFS (patches 1 - 12):

 fs/Kconfig                                |    3
 fs/nfs/Kconfig                            |   14
 fs/nfs/Makefile                           |    1
 fs/nfs/blocklayout/blocklayout.c          |    6
 fs/nfs/client.c                           |   15
 fs/nfs/filelayout/filelayout.c            |   16
 fs/nfs/flexfilelayout/flexfilelayout.c    |  131 ++++
 fs/nfs/flexfilelayout/flexfilelayout.h    |    2
 fs/nfs/flexfilelayout/flexfilelayoutdev.c |    6
 fs/nfs/inode.c                            |    4
 fs/nfs/internal.h                         |   60 ++
 fs/nfs/localio.c                          |  827 ++++++++++++++++++++++++++++++
 fs/nfs/nfs4xdr.c                          |   13
 fs/nfs/nfstrace.h                         |   61 ++
 fs/nfs/pagelist.c                         |   32 -
 fs/nfs/pnfs.c                             |   24
 fs/nfs/pnfs.h                             |    6
 fs/nfs/pnfs_nfs.c                         |    2
 fs/nfs/write.c                            |   13
 fs/nfs_common/Makefile                    |    3
 fs/nfs_common/nfslocalio.c                |   74 ++
 fs/nfsd/netns.h                           |    4
 fs/nfsd/nfssvc.c                          |   12
 include/linux/nfs.h                       |    9
 include/linux/nfs_fs.h                    |    2
 include/linux/nfs_fs_sb.h                 |   10
 include/linux/nfs_xdr.h                   |   20
 include/linux/nfslocalio.h                |   39 +
 include/linux/sunrpc/auth.h               |    4
 net/sunrpc/auth.c                         |   15
 net/sunrpc/clnt.c                         |    1
 31 files changed, 1354 insertions(+), 75 deletions(-)

Unfortunately there are the fs/nfsd/netns.h and fs/nfsd/nfssvc.c
changes that anchor everything (patch 5).

I suppose we could invert the order, such that NFSD comes before NFS
changes.  But then the NFS tree will need to be rebased on NFSD tree.

Diffstat for NFSD (patches 13 - 19):

 Documentation/filesystems/nfs/localio.rst |  135 ++++++++++++
 fs/nfsd/Kconfig                           |   14 +
 fs/nfsd/Makefile                          |    1 
 fs/nfsd/filecache.c                       |    2 
 fs/nfsd/localio.c                         |  319 ++++++++++++++++++++++++++++++
 fs/nfsd/netns.h                           |    8 
 fs/nfsd/nfsctl.c                          |    2 
 fs/nfsd/nfsd.h                            |    2 
 fs/nfsd/nfssvc.c                          |  104 +++++++--
 fs/nfsd/trace.h                           |    3 
 fs/nfsd/vfs.h                             |    9 
 include/linux/nfslocalio.h                |    2 
 include/linux/sunrpc/svc.h                |    7 
 net/sunrpc/svc.c                          |   68 +++---
 net/sunrpc/svc_xprt.c                     |    2 
 net/sunrpc/svcauth_unix.c                 |    3 
 16 files changed, 621 insertions(+), 60 deletions(-)

Happy to work it however you think is best.

> > Worked with Kent Overstreet to enable testing integration with ktest
> > running xfstests, the dashboard is here:
> > https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
> > (it is running way more xfstests tests than is usual for nfs, would be
> > good to reconcile that with the listing provided here:
> > https://wiki.linux-nfs.org/wiki/index.php/Xfstests )
> 
> Actually, we're using kdevops for NFSD CI testing. Any possibility
> that we can get some help setting that up? (It runs xfstests and
> several other workflows).

Sure, I can get with you off-list if that's best?  I just need some
pointers/access to help get it going.

Thanks,
Mike
Chuck Lever III June 29, 2024, 5:01 p.m. UTC | #3
On Sat, Jun 29, 2024 at 12:03:50PM -0400, Mike Snitzer wrote:
> On Sat, Jun 29, 2024 at 03:36:10PM +0000, Chuck Lever III wrote:
> > 
> > 
> > > On Jun 28, 2024, at 5:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
> > > 
> > > Hi,
> > > 
> > > I'd prefer to see these changes land upstream for 6.11 if possible.
> > > They are adequately Kconfig'd to certainly pose no risk if disabled.
> > > And even if localio enabled it has proven to work well with increased
> > > testing.
> > 
> > Can v10 split this series into an NFS client part and an NFS
> > server part? I will need to get the NFSD changes into nfsd-next
> > in the next week or so to land in v6.11.
> 
> I forgot to mention this as a v9 improvement: I did split the series,
> but left it as one patchset.
> 
> Patches 1-12 are NFS client, Patches 13-19 are NFSD.

I didn't notice that because my MUA displayed the patches completely
out of order. Apologies!


> Here is the diffstat for NFS (patches 1 - 12):
> 
>  fs/Kconfig                                |    3
>  fs/nfs/Kconfig                            |   14
>  fs/nfs/Makefile                           |    1
>  fs/nfs/blocklayout/blocklayout.c          |    6
>  fs/nfs/client.c                           |   15
>  fs/nfs/filelayout/filelayout.c            |   16
>  fs/nfs/flexfilelayout/flexfilelayout.c    |  131 ++++
>  fs/nfs/flexfilelayout/flexfilelayout.h    |    2
>  fs/nfs/flexfilelayout/flexfilelayoutdev.c |    6
>  fs/nfs/inode.c                            |    4
>  fs/nfs/internal.h                         |   60 ++
>  fs/nfs/localio.c                          |  827 ++++++++++++++++++++++++++++++
>  fs/nfs/nfs4xdr.c                          |   13
>  fs/nfs/nfstrace.h                         |   61 ++
>  fs/nfs/pagelist.c                         |   32 -
>  fs/nfs/pnfs.c                             |   24
>  fs/nfs/pnfs.h                             |    6
>  fs/nfs/pnfs_nfs.c                         |    2
>  fs/nfs/write.c                            |   13
>  fs/nfs_common/Makefile                    |    3
>  fs/nfs_common/nfslocalio.c                |   74 ++
>  fs/nfsd/netns.h                           |    4
>  fs/nfsd/nfssvc.c                          |   12
>  include/linux/nfs.h                       |    9
>  include/linux/nfs_fs.h                    |    2
>  include/linux/nfs_fs_sb.h                 |   10
>  include/linux/nfs_xdr.h                   |   20
>  include/linux/nfslocalio.h                |   39 +
>  include/linux/sunrpc/auth.h               |    4
>  net/sunrpc/auth.c                         |   15
>  net/sunrpc/clnt.c                         |    1
>  31 files changed, 1354 insertions(+), 75 deletions(-)
> 
> Unfortunately there are the fs/nfsd/netns.h and fs/nfsd/nfssvc.c
> changes that anchor everything (patch 5).

I /did/ notice that.


> I suppose we could invert the order, such that NFSD comes before NFS
> changes.  But then the NFS tree will need to be rebased on NFSD tree.

Alternately, I can take the NFSD-related patches in 6.11, and the
client changes can go in 6.12. My impression (could be wrong) was
that the NFSD patches were nearly ready but the client side was
still churning a little.

Or we might decide that it's not worth the trouble. Anna offered to
take the whole series, or I can. If Anna takes it, I'll send
Acked-by for the NFSD patches.


> Diffstat for NFSD (patches 13 - 19):
> 
>  Documentation/filesystems/nfs/localio.rst |  135 ++++++++++++
>  fs/nfsd/Kconfig                           |   14 +
>  fs/nfsd/Makefile                          |    1 
>  fs/nfsd/filecache.c                       |    2 
>  fs/nfsd/localio.c                         |  319 ++++++++++++++++++++++++++++++
>  fs/nfsd/netns.h                           |    8 
>  fs/nfsd/nfsctl.c                          |    2 
>  fs/nfsd/nfsd.h                            |    2 
>  fs/nfsd/nfssvc.c                          |  104 +++++++--
>  fs/nfsd/trace.h                           |    3 
>  fs/nfsd/vfs.h                             |    9 
>  include/linux/nfslocalio.h                |    2 
>  include/linux/sunrpc/svc.h                |    7 
>  net/sunrpc/svc.c                          |   68 +++---
>  net/sunrpc/svc_xprt.c                     |    2 
>  net/sunrpc/svcauth_unix.c                 |    3 
>  16 files changed, 621 insertions(+), 60 deletions(-)
> 
> Happy to work it however you think is best.
> 
> > > Worked with Kent Overstreet to enable testing integration with ktest
> > > running xfstests, the dashboard is here:
> > > https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
> > > (it is running way more xfstests tests than is usual for nfs, would be
> > > good to reconcile that with the listing provided here:
> > > https://wiki.linux-nfs.org/wiki/index.php/Xfstests )
> > 
> > Actually, we're using kdevops for NFSD CI testing. Any possibility
> > that we can get some help setting that up? (It runs xfstests and
> > several other workflows).
> 
> Sure, I can get with you off-list if that's best?  I just need some
> pointers/access to help get it going.

Yes, off-list wfm.

Come to think of it, it might just work to point my test systems to
your git branch and let it rip, if there are no new tests. I will
try that.
Mike Snitzer June 29, 2024, 7:10 p.m. UTC | #4
On Sat, Jun 29, 2024 at 01:01:57PM -0400, Chuck Lever wrote:
> On Sat, Jun 29, 2024 at 12:03:50PM -0400, Mike Snitzer wrote:
> > On Sat, Jun 29, 2024 at 03:36:10PM +0000, Chuck Lever III wrote:
> > > 
> > > 
> > > > On Jun 28, 2024, at 5:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > I'd prefer to see these changes land upstream for 6.11 if possible.
> > > > They are adequately Kconfig'd to certainly pose no risk if disabled.
> > > > And even if localio enabled it has proven to work well with increased
> > > > testing.
> > > 
> > > Can v10 split this series into an NFS client part and an NFS
> > > server part? I will need to get the NFSD changes into nfsd-next
> > > in the next week or so to land in v6.11.
> > 
> > I forgot to mention this as a v9 improvement: I did split the series,
> > but left it as one patchset.
> > 
> > Patches 1-12 are NFS client, Patches 13-19 are NFSD.
> 
> I didn't notice that because my MUA displayed the patches completely
> out of order. Apologies!
>
> > Here is the diffstat for NFS (patches 1 - 12):
> > 
> >  fs/Kconfig                                |    3
> >  fs/nfs/Kconfig                            |   14
> >  fs/nfs/Makefile                           |    1
> >  fs/nfs/blocklayout/blocklayout.c          |    6
> >  fs/nfs/client.c                           |   15
> >  fs/nfs/filelayout/filelayout.c            |   16
> >  fs/nfs/flexfilelayout/flexfilelayout.c    |  131 ++++
> >  fs/nfs/flexfilelayout/flexfilelayout.h    |    2
> >  fs/nfs/flexfilelayout/flexfilelayoutdev.c |    6
> >  fs/nfs/inode.c                            |    4
> >  fs/nfs/internal.h                         |   60 ++
> >  fs/nfs/localio.c                          |  827 ++++++++++++++++++++++++++++++
> >  fs/nfs/nfs4xdr.c                          |   13
> >  fs/nfs/nfstrace.h                         |   61 ++
> >  fs/nfs/pagelist.c                         |   32 -
> >  fs/nfs/pnfs.c                             |   24
> >  fs/nfs/pnfs.h                             |    6
> >  fs/nfs/pnfs_nfs.c                         |    2
> >  fs/nfs/write.c                            |   13
> >  fs/nfs_common/Makefile                    |    3
> >  fs/nfs_common/nfslocalio.c                |   74 ++
> >  fs/nfsd/netns.h                           |    4
> >  fs/nfsd/nfssvc.c                          |   12
> >  include/linux/nfs.h                       |    9
> >  include/linux/nfs_fs.h                    |    2
> >  include/linux/nfs_fs_sb.h                 |   10
> >  include/linux/nfs_xdr.h                   |   20
> >  include/linux/nfslocalio.h                |   39 +
> >  include/linux/sunrpc/auth.h               |    4
> >  net/sunrpc/auth.c                         |   15
> >  net/sunrpc/clnt.c                         |    1
> >  31 files changed, 1354 insertions(+), 75 deletions(-)
> > 
> > Unfortunately there are the fs/nfsd/netns.h and fs/nfsd/nfssvc.c
> > changes that anchor everything (patch 5).
> 
> I /did/ notice that.
> 
> 
> > I suppose we could invert the order, such that NFSD comes before NFS
> > changes.  But then the NFS tree will need to be rebased on NFSD tree.
> 
> Alternately, I can take the NFSD-related patches in 6.11, and the
> client changes can go in 6.12. My impression (could be wrong) was
> that the NFSD patches were nearly ready but the client side was
> still churning a little.

I'm "done" with both afaik.  Only thing that needs settling is that
XFS RFC patch I posted.

> Or we might decide that it's not worth the trouble. Anna offered to
> take the whole series, or I can. If Anna takes it, I'll send
> Acked-by for the NFSD patches.

Probably best to have it all go through the same tree.  Just get proper
Acked-by:s where needed.

I would say it is more client heavy (in terms of code foot-print) so
maybe it does make more sense to go through NFS.  Anna is handling the
6.11 merge for NFS so let's just work on getting proper Acked-by.

If you, Jeff and Neil could do a final review and provide Acked-by (or
conditional Acked-by if I fold your suggestions in for v10) I'll add
all your final feedback and Acked-by:s or Reviewed-by:s so Anna will
be able to simply pick it up once the NFS client side is also
reviewed.

> > Diffstat for NFSD (patches 13 - 19):
> > 
> >  Documentation/filesystems/nfs/localio.rst |  135 ++++++++++++
> >  fs/nfsd/Kconfig                           |   14 +
> >  fs/nfsd/Makefile                          |    1 
> >  fs/nfsd/filecache.c                       |    2 
> >  fs/nfsd/localio.c                         |  319 ++++++++++++++++++++++++++++++
> >  fs/nfsd/netns.h                           |    8 
> >  fs/nfsd/nfsctl.c                          |    2 
> >  fs/nfsd/nfsd.h                            |    2 
> >  fs/nfsd/nfssvc.c                          |  104 +++++++--
> >  fs/nfsd/trace.h                           |    3 
> >  fs/nfsd/vfs.h                             |    9 
> >  include/linux/nfslocalio.h                |    2 
> >  include/linux/sunrpc/svc.h                |    7 
> >  net/sunrpc/svc.c                          |   68 +++---
> >  net/sunrpc/svc_xprt.c                     |    2 
> >  net/sunrpc/svcauth_unix.c                 |    3 
> >  16 files changed, 621 insertions(+), 60 deletions(-)
> > 
> > Happy to work it however you think is best.
> > 
> > > > Worked with Kent Overstreet to enable testing integration with ktest
> > > > running xfstests, the dashboard is here:
> > > > https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
> > > > (it is running way more xfstests tests than is usual for nfs, would be
> > > > good to reconcile that with the listing provided here:
> > > > https://wiki.linux-nfs.org/wiki/index.php/Xfstests )
> > > 
> > > Actually, we're using kdevops for NFSD CI testing. Any possibility
> > > that we can get some help setting that up? (It runs xfstests and
> > > several other workflows).
> > 
> > Sure, I can get with you off-list if that's best?  I just need some
> > pointers/access to help get it going.
> 
> Yes, off-list wfm.
> 
> Come to think of it, it might just work to point my test systems to
> your git branch and let it rip, if there are no new tests. I will
> try that.

Right, no new tests added to xfstests, it was purely configuration to
get xfstests running on single host in loopback mode (NFS client
mounting export from knfsd on same host).

Would be great if you could point your kdevops at my localio-for-6.11
branch.  You just need to make sure to enable these in your Kconfig:

CONFIG_NFSD_LOCALIO=y
CONFIG_NFS_LOCALIO=y
CONFIG_NFS_COMMON_LOCALIO_SUPPORT=y

(either of the NFS or NFSD options will select
CONFIG_NFS_COMMON_LOCALIO_SUPPORT)
Chuck Lever III June 29, 2024, 8:31 p.m. UTC | #5
> On Jun 29, 2024, at 3:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
> 
> On Sat, Jun 29, 2024 at 01:01:57PM -0400, Chuck Lever wrote:
>> On Sat, Jun 29, 2024 at 12:03:50PM -0400, Mike Snitzer wrote:
>>> On Sat, Jun 29, 2024 at 03:36:10PM +0000, Chuck Lever III wrote:
>>>> 
>>>> 
>>>>> On Jun 28, 2024, at 5:10 PM, Mike Snitzer <snitzer@kernel.org> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> I'd prefer to see these changes land upstream for 6.11 if possible.
>>>>> They are adequately Kconfig'd to certainly pose no risk if disabled.
>>>>> And even if localio enabled it has proven to work well with increased
>>>>> testing.
>>>> 
>>>> Can v10 split this series into an NFS client part and an NFS
>>>> server part? I will need to get the NFSD changes into nfsd-next
>>>> in the next week or so to land in v6.11.
>>> 
>>> I forgot to mention this as a v9 improvement: I did split the series,
>>> but left it as one patchset.
>>> 
>>> Patches 1-12 are NFS client, Patches 13-19 are NFSD.
>> 
>> I didn't notice that because my MUA displayed the patches completely
>> out of order. Apologies!
>> 
>>> Here is the diffstat for NFS (patches 1 - 12):
>>> 
>>> fs/Kconfig                                |    3
>>> fs/nfs/Kconfig                            |   14
>>> fs/nfs/Makefile                           |    1
>>> fs/nfs/blocklayout/blocklayout.c          |    6
>>> fs/nfs/client.c                           |   15
>>> fs/nfs/filelayout/filelayout.c            |   16
>>> fs/nfs/flexfilelayout/flexfilelayout.c    |  131 ++++
>>> fs/nfs/flexfilelayout/flexfilelayout.h    |    2
>>> fs/nfs/flexfilelayout/flexfilelayoutdev.c |    6
>>> fs/nfs/inode.c                            |    4
>>> fs/nfs/internal.h                         |   60 ++
>>> fs/nfs/localio.c                          |  827 ++++++++++++++++++++++++++++++
>>> fs/nfs/nfs4xdr.c                          |   13
>>> fs/nfs/nfstrace.h                         |   61 ++
>>> fs/nfs/pagelist.c                         |   32 -
>>> fs/nfs/pnfs.c                             |   24
>>> fs/nfs/pnfs.h                             |    6
>>> fs/nfs/pnfs_nfs.c                         |    2
>>> fs/nfs/write.c                            |   13
>>> fs/nfs_common/Makefile                    |    3
>>> fs/nfs_common/nfslocalio.c                |   74 ++
>>> fs/nfsd/netns.h                           |    4
>>> fs/nfsd/nfssvc.c                          |   12
>>> include/linux/nfs.h                       |    9
>>> include/linux/nfs_fs.h                    |    2
>>> include/linux/nfs_fs_sb.h                 |   10
>>> include/linux/nfs_xdr.h                   |   20
>>> include/linux/nfslocalio.h                |   39 +
>>> include/linux/sunrpc/auth.h               |    4
>>> net/sunrpc/auth.c                         |   15
>>> net/sunrpc/clnt.c                         |    1
>>> 31 files changed, 1354 insertions(+), 75 deletions(-)
>>> 
>>> Unfortunately there are the fs/nfsd/netns.h and fs/nfsd/nfssvc.c
>>> changes that anchor everything (patch 5).
>> 
>> I /did/ notice that.
>> 
>> 
>>> I suppose we could invert the order, such that NFSD comes before NFS
>>> changes.  But then the NFS tree will need to be rebased on NFSD tree.
>> 
>> Alternately, I can take the NFSD-related patches in 6.11, and the
>> client changes can go in 6.12. My impression (could be wrong) was
>> that the NFSD patches were nearly ready but the client side was
>> still churning a little.
> 
> I'm "done" with both afaik.  Only thing that needs settling is that
> XFS RFC patch I posted.
> 
>> Or we might decide that it's not worth the trouble. Anna offered to
>> take the whole series, or I can. If Anna takes it, I'll send
>> Acked-by for the NFSD patches.
> 
> Probably best to have it all go through the same tree.  Just get proper
> Acked-by:s where needed.
> 
> I would say it is more client heavy (in terms of code foot-print) so
> maybe it does make more sense to go through NFS.  Anna is handling the
> 6.11 merge for NFS so let's just work on getting proper Acked-by.
> 
> If you, Jeff and Neil could do a final review and provide Acked-by (or
> conditional Acked-by if I fold your suggestions in for v10) I'll add
> all your final feedback and Acked-by:s or Reviewed-by:s so Anna will
> be able to simply pick it up once the NFS client side is also
> reviewed.

Anna suggested this should soak in linux-next until v6.12.
I don't have a strong preference, though v6.12 seems like
a safer goal if you haven't seen any client-side review yet.


>>> Diffstat for NFSD (patches 13 - 19):
>>> 
>>> Documentation/filesystems/nfs/localio.rst |  135 ++++++++++++
>>> fs/nfsd/Kconfig                           |   14 +
>>> fs/nfsd/Makefile                          |    1 
>>> fs/nfsd/filecache.c                       |    2 
>>> fs/nfsd/localio.c                         |  319 ++++++++++++++++++++++++++++++
>>> fs/nfsd/netns.h                           |    8 
>>> fs/nfsd/nfsctl.c                          |    2 
>>> fs/nfsd/nfsd.h                            |    2 
>>> fs/nfsd/nfssvc.c                          |  104 +++++++--
>>> fs/nfsd/trace.h                           |    3 
>>> fs/nfsd/vfs.h                             |    9 
>>> include/linux/nfslocalio.h                |    2 
>>> include/linux/sunrpc/svc.h                |    7 
>>> net/sunrpc/svc.c                          |   68 +++---
>>> net/sunrpc/svc_xprt.c                     |    2 
>>> net/sunrpc/svcauth_unix.c                 |    3 
>>> 16 files changed, 621 insertions(+), 60 deletions(-)
>>> 
>>> Happy to work it however you think is best.
>>> 
>>>>> Worked with Kent Overstreet to enable testing integration with ktest
>>>>> running xfstests, the dashboard is here:
>>>>> https://evilpiepirate.org/~testdashboard/ci?branch=snitm-nfs
>>>>> (it is running way more xfstests tests than is usual for nfs, would be
>>>>> good to reconcile that with the listing provided here:
>>>>> https://wiki.linux-nfs.org/wiki/index.php/Xfstests )
>>>> 
>>>> Actually, we're using kdevops for NFSD CI testing. Any possibility
>>>> that we can get some help setting that up? (It runs xfstests and
>>>> several other workflows).
>>> 
>>> Sure, I can get with you off-list if that's best?  I just need some
>>> pointers/access to help get it going.
>> 
>> Yes, off-list wfm.
>> 
>> Come to think of it, it might just work to point my test systems to
>> your git branch and let it rip, if there are no new tests. I will
>> try that.
> 
> Right, no new tests added to xfstests, it was purely configuration to
> get xfstests running on single host in loopback mode (NFS client
> mounting export from knfsd on same host).
> 
> Would be great if you could point your kdevops at my localio-for-6.11
> branch.  You just need to make sure to enable these in your Kconfig:
> 
> CONFIG_NFSD_LOCALIO=y
> CONFIG_NFS_LOCALIO=y
> CONFIG_NFS_COMMON_LOCALIO_SUPPORT=y
> 
> (either of the NFS or NFSD options will select
> CONFIG_NFS_COMMON_LOCALIO_SUPPORT)

I'm running the first set right now. We don't have a public
dashboard yet, but I can set up a MailNotifier for you.

You don't have any metrics that show whether (and how many)
local read and write operations are happening; so I can
tell if tests pass or fail, but not if local I/O is going
on. The usual approach is to hook that kind of client
metric into /proc/self/mountstats.


--
Chuck Lever