mbox series

[v4,00/18] nfs/nfsd: add support for localio

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

Message

Mike Snitzer June 18, 2024, 1:08 a.m. UTC
Hi,

This v4 fixes a few bugs in v3, reorders patches and improves patch
headers and code documentation. Please pay particular attention to
patches 17 and 18.

If all looks good to others, for v5 I can rebase to the -next trees
for nfs and nfsd.

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

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

nfs-localio-for-6.11.v3, nfs-localio-for-6.11.v2 and
nfs-localio-for-6.11.v1 are also there.

To see the changes from v3 to v4 please do:
git remote add snitzer git://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git
git remote update snitzer
git diff snitzer/nfs-localio-for-6.11.v3 snitzer/nfs-localio-for-6.11.v4

These changes have proven stable against various test scenarios:
1) client and server both on localhost (for both v3 and v4.2)
2) various permutations of client and server support enablement for
   both local and remote client and server.
3) client on host, server within a container (for both v3 and v4.2)
   My container testing was in terms of podman managed containers.
4) container stop/restart scenario documented in the last patch

All review and comments are welcome!

Thanks,
Mike

Mike Snitzer (10):
  nfs_common: add NFS LOCALIO protocol extension enablement
  nfs: implement v3 and v4 client support for NFS_LOCALIO_PROGRAM
  nfsd: implement v3 and v4 server support for NFS_LOCALIO_PROGRAM
  nfs/nfsd: consolidate {encode,decode}_opaque_fixed in nfs_xdr.h
  nfs/localio: move managing nfsd_open_local_fh symbol to nfs_common
  nfs/nfsd: ensure localio server always uses its network namespace
  nfsd/localio: manage netns reference in nfsd_open_local_fh
  nfsd: prepare to use SRCU to dereference nn->nfsd_serv
  nfsd: use SRCU to dereference nn->nfsd_serv
  nfsd/localio: use SRCU to dereference nn->nfsd_serv in nfsd_open_local_fh

Trond Myklebust (3):
  NFS: Enable localio for non-pNFS I/O
  pnfs/flexfiles: Enable localio for flexfiles I/O
  nfs/localio: use dedicated workqueues for filesystem read and write

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/nfsd: add "localio" support

 fs/Kconfig                                |   3 +
 fs/nfs/Kconfig                            |  30 +
 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                            |  61 +-
 fs/nfs/internal.h                         |  88 ++-
 fs/nfs/localio.c                          | 850 ++++++++++++++++++++++
 fs/nfs/nfs3_fs.h                          |   1 +
 fs/nfs/nfs3client.c                       |  25 +
 fs/nfs/nfs3proc.c                         |   3 +
 fs/nfs/nfs3xdr.c                          |  58 ++
 fs/nfs/nfs4_fs.h                          |   2 +
 fs/nfs/nfs4client.c                       |  23 +
 fs/nfs/nfs4proc.c                         |   3 +
 fs/nfs/nfs4xdr.c                          |  65 +-
 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                |  71 ++
 fs/nfsd/Kconfig                           |  30 +
 fs/nfsd/Makefile                          |   1 +
 fs/nfsd/filecache.c                       |  15 +-
 fs/nfsd/localio.c                         | 398 ++++++++++
 fs/nfsd/netns.h                           |  16 +-
 fs/nfsd/nfs4state.c                       |  25 +-
 fs/nfsd/nfsctl.c                          |  28 +-
 fs/nfsd/nfsd.h                            |  11 +
 fs/nfsd/nfssvc.c                          | 176 ++++-
 fs/nfsd/trace.h                           |   3 +-
 fs/nfsd/vfs.h                             |   9 +
 fs/nfsd/xdr.h                             |   6 +
 include/linux/nfs.h                       |   2 +
 include/linux/nfs_fs.h                    |   2 +
 include/linux/nfs_fs_sb.h                 |   9 +
 include/linux/nfs_xdr.h                   |  31 +-
 include/linux/nfslocalio.h                |  61 ++
 include/linux/sunrpc/auth.h               |   4 +
 include/uapi/linux/nfs.h                  |   4 +
 net/sunrpc/auth.c                         |  15 +
 48 files changed, 2305 insertions(+), 142 deletions(-)
 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 June 18, 2024, 2:25 p.m. UTC | #1
On Mon, Jun 17, 2024 at 09:08:59PM -0400, Mike Snitzer wrote:
> Hi,
> 
> This v4 fixes a few bugs in v3, reorders patches and improves patch
> headers and code documentation. Please pay particular attention to
> patches 17 and 18.
> 
> If all looks good to others, for v5 I can rebase to the -next trees
> for nfs and nfsd.
> 
> My git tree is here:
> https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/
> 
> This v4 is both branch nfs-localio-for-6.11 (always tracks latest)
> and nfs-localio-for-6.11.v4
> 
> nfs-localio-for-6.11.v3, nfs-localio-for-6.11.v2 and
> nfs-localio-for-6.11.v1 are also there.
> 
> To see the changes from v3 to v4 please do:
> git remote add snitzer git://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git
> git remote update snitzer
> git diff snitzer/nfs-localio-for-6.11.v3 snitzer/nfs-localio-for-6.11.v4
> 
> These changes have proven stable against various test scenarios:
> 1) client and server both on localhost (for both v3 and v4.2)
> 2) various permutations of client and server support enablement for
>    both local and remote client and server.
> 3) client on host, server within a container (for both v3 and v4.2)
>    My container testing was in terms of podman managed containers.
> 4) container stop/restart scenario documented in the last patch
> 
> All review and comments are welcome!

With my NFSD maintainer hat on: I'm concerned about some of the
long-term maintenance work being added by this series. This is
more of a concern on the client side, for sure, but, IMO:

In a perfect world, we would have an RFC for this, and the set
would come with tests we can add to our release CI framework. I'm
not holding you to all that, but I would like to see something to
help me out in the long-run.

Because this is not part of an Internet standard, this patch set
needs to come with some architectural documentation like something
under Documentation/filesystems/nfs.

It needs to explain the use cases and why this design was chosen.
It should have a specification for the LOCALIO protocol. It needs
to explain how to test the facility being added -- basically we
need to know how /not/ to break this thing as we develop around it.
I'm also interested to know who, besides Hammerspace, can benefit
from this facility.

(This isn't a hard objection, just a request for some help with
the long-term maintenance burden).
Mike Snitzer June 18, 2024, 4:15 p.m. UTC | #2
On Tue, Jun 18, 2024 at 10:25:19AM -0400, Chuck Lever wrote:
> On Mon, Jun 17, 2024 at 09:08:59PM -0400, Mike Snitzer wrote:
> > Hi,
> > 
> > This v4 fixes a few bugs in v3, reorders patches and improves patch
> > headers and code documentation. Please pay particular attention to
> > patches 17 and 18.
> > 
> > If all looks good to others, for v5 I can rebase to the -next trees
> > for nfs and nfsd.
> > 
> > My git tree is here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/
> > 
> > This v4 is both branch nfs-localio-for-6.11 (always tracks latest)
> > and nfs-localio-for-6.11.v4
> > 
> > nfs-localio-for-6.11.v3, nfs-localio-for-6.11.v2 and
> > nfs-localio-for-6.11.v1 are also there.
> > 
> > To see the changes from v3 to v4 please do:
> > git remote add snitzer git://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git
> > git remote update snitzer
> > git diff snitzer/nfs-localio-for-6.11.v3 snitzer/nfs-localio-for-6.11.v4
> > 
> > These changes have proven stable against various test scenarios:
> > 1) client and server both on localhost (for both v3 and v4.2)
> > 2) various permutations of client and server support enablement for
> >    both local and remote client and server.
> > 3) client on host, server within a container (for both v3 and v4.2)
> >    My container testing was in terms of podman managed containers.
> > 4) container stop/restart scenario documented in the last patch
> > 
> > All review and comments are welcome!
> 
> With my NFSD maintainer hat on: I'm concerned about some of the
> long-term maintenance work being added by this series. This is
> more of a concern on the client side, for sure, but, IMO:
> 
> In a perfect world, we would have an RFC for this, and the set
> would come with tests we can add to our release CI framework. I'm
> not holding you to all that, but I would like to see something to
> help me out in the long-run.

I understand.  Thankfully it is quite easy to test with client and
server running on the same host.

The container-based testing (e.g. running client on host, nfsd in
container) is a bit more fiddley/specialized to get setup due to the
nature of all things containers.

> Because this is not part of an Internet standard, this patch set
> needs to come with some architectural documentation like something
> under Documentation/filesystems/nfs.
> 
> It needs to explain the use cases and why this design was chosen.
> It should have a specification for the LOCALIO protocol. It needs
> to explain how to test the facility being added -- basically we
> need to know how /not/ to break this thing as we develop around it.
> I'm also interested to know who, besides Hammerspace, can benefit
> from this facility.
> 
> (This isn't a hard objection, just a request for some help with
> the long-term maintenance burden).

Sure, I'll work on Documentation for v5. These changes help everyone
who might like to run client and server on the same system (and within
containers). That may not be interesting to a lot of people but it
isn't soooo niche.

As for maintenance burden: thankfully these changes are mostly
isolated from the bulk of the rest of the nfs and nfsd code.

The SRCU changes to nfsd less so, but they are largely mechaical and
straight-forward (when I ported to your latest nfsd-next last night,
there was a simple matter of needing to fixup naked nn->nfsd_serv
dereferences that were caught by the compiler due to dereferencing a
void pointer warnings).

Thanks,
Mike