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

[Mike Snitzer <snitzer@kernel.org>]

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