> On Jun 29, 2024, at 3:10 PM, Mike Snitzer <snitzer@xxxxxxxxxx> 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@xxxxxxxxxx> 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
