Hi Mike, On 1/8/25 11:05 AM, Mike Snitzer wrote: > [top-posting because of my general inquiry] > > Hi Anna, > > I know Trond has been quite busy, I suspect you have been too > (holidays and all too). I just wanted to check with you on this > patchset. > > Given the 6.14 merge window is fast approaching, is there anything you > need from me? > > Jeff did provide his Reviewed-by, and Chuck his Acked-by. But would > it help for me to prepare a v4 that explicitly adds their tags > accordingly? Thanks for checking in! I don't think I need anything from you at this time. I just pushed out a linux-next branch including these patches and your additional bugfix from the other week, so everything should be covered. If you notice anything missing, do let me know! Anna > > There is also this LOCALIO error path fix that should go upstream and > be marked for stable@: > https://lore.kernel.org/linux-nfs/20241227201356.3074-1-snitzer@xxxxxxxxxx/ > > I could put that fix in the front of a v4 resubmission. I'm also > perfectly fine to stand-down and let you review and apply if/when you > have time.. the patches haven't changed at all, but happy to sweep > through and do the v4 if it helps. > > Thanks, > Mike > > > On Wed, Dec 18, 2024 at 04:19:42PM -0500, Mike Snitzer wrote: >> On Wed, Dec 18, 2024 at 04:05:23PM -0500, Chuck Lever wrote: >>> On 12/18/24 3:55 PM, Anna Schumaker wrote: >>>> >>>> >>>> On 12/18/24 12:31 PM, Mike Snitzer wrote: >>>>> On Fri, Nov 22, 2024 at 09:31:23PM -0500, Mike Snitzer wrote: >>>>>> On Fri, Nov 22, 2024 at 12:26:39PM -0500, Jeff Layton wrote: >>>>>>> On Fri, 2024-11-15 at 20:40 -0500, Mike Snitzer wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> All available here: >>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next >>>>>>>> >>>>>>>> Changes since v2: >>>>>>>> - switched from rcu_assign_pointer to RCU_INIT_POINTER when setting to >>>>>>>> NULL. >>>>>>>> - removed some unnecessary #if IS_ENABLED(CONFIG_NFS_LOCALIO) >>>>>>>> - revised the NFS v3 probe patch to use a new nfsv3.ko modparam >>>>>>>> 'nfs3_localio_probe_throttle' to control if NFSv3 will probe for >>>>>>>> LOCALIO. Avoids use of NFS_CS_LOCAL_IO and will probe every >>>>>>>> 'nfs3_localio_probe_throttle' IO requests (defaults to 0, disabled). >>>>>>>> - added "Module Parameters" section to localio.rst >>>>>>>> >>>>>>>> All review appreciated, thanks. >>>>>>>> Mike >>>>>>>> >>>>>>>> Mike Snitzer (14): >>>>>>>> nfs/localio: add direct IO enablement with sync and async IO support >>>>>>>> nfsd: add nfsd_file_{get,put} to 'nfs_to' nfsd_localio_operations >>>>>>>> nfs_common: rename functions that invalidate LOCALIO nfs_clients >>>>>>>> nfs_common: move localio_lock to new lock member of nfs_uuid_t >>>>>>>> nfs: cache all open LOCALIO nfsd_file(s) in client >>>>>>>> nfsd: update percpu_ref to manage references on nfsd_net >>>>>>>> nfsd: rename nfsd_serv_ prefixed methods and variables with nfsd_net_ >>>>>>>> nfsd: nfsd_file_acquire_local no longer returns GC'd nfsd_file >>>>>>>> nfs_common: rename nfslocalio nfs_uuid_lock to nfs_uuids_lock >>>>>>>> nfs_common: track all open nfsd_files per LOCALIO nfs_client >>>>>>>> nfs_common: add nfs_localio trace events >>>>>>>> nfs/localio: remove redundant code and simplify LOCALIO enablement >>>>>>>> nfs: probe for LOCALIO when v4 client reconnects to server >>>>>>>> nfs: probe for LOCALIO when v3 client reconnects to server >>>>>>>> >>>>>>>> Documentation/filesystems/nfs/localio.rst | 98 +++++---- >>>>>>>> fs/nfs/client.c | 6 +- >>>>>>>> fs/nfs/direct.c | 1 + >>>>>>>> fs/nfs/flexfilelayout/flexfilelayout.c | 25 +-- >>>>>>>> fs/nfs/flexfilelayout/flexfilelayout.h | 1 + >>>>>>>> fs/nfs/inode.c | 3 + >>>>>>>> fs/nfs/internal.h | 9 +- >>>>>>>> fs/nfs/localio.c | 232 +++++++++++++++----- >>>>>>>> fs/nfs/nfs3proc.c | 46 +++- >>>>>>>> fs/nfs/nfs4state.c | 1 + >>>>>>>> fs/nfs/nfstrace.h | 32 --- >>>>>>>> fs/nfs/pagelist.c | 5 +- >>>>>>>> fs/nfs/write.c | 3 +- >>>>>>>> fs/nfs_common/Makefile | 3 +- >>>>>>>> fs/nfs_common/localio_trace.c | 10 + >>>>>>>> fs/nfs_common/localio_trace.h | 56 +++++ >>>>>>>> fs/nfs_common/nfslocalio.c | 250 +++++++++++++++++----- >>>>>>>> fs/nfsd/filecache.c | 20 +- >>>>>>>> fs/nfsd/localio.c | 9 +- >>>>>>>> fs/nfsd/netns.h | 12 +- >>>>>>>> fs/nfsd/nfsctl.c | 6 +- >>>>>>>> fs/nfsd/nfssvc.c | 40 ++-- >>>>>>>> include/linux/nfs_fs.h | 22 +- >>>>>>>> include/linux/nfs_fs_sb.h | 3 +- >>>>>>>> include/linux/nfs_xdr.h | 1 + >>>>>>>> include/linux/nfslocalio.h | 48 +++-- >>>>>>>> 26 files changed, 674 insertions(+), 268 deletions(-) >>>>>>>> create mode 100644 fs/nfs_common/localio_trace.c >>>>>>>> create mode 100644 fs/nfs_common/localio_trace.h >>>>>>>> >>>>>>> >>>>>>> I went through the set and it looks mostly sane to me. The one concern >>>>>>> I have is that you have the client set up to start caching nfsd files >>>>>>> before there is a mechanism to call it and ask them to return them. You >>>>>>> might see some weird behavior there on a bisect, but it looks like it >>>>>>> all gets resolved in the end. >>>>>> >>>>>> Yeah, couldn't see a better way to atomically pivot to the new disable >>>>>> functionality without it needing to be a large muddled patch. >>>>>> >>>>>> Shouldn't be bad even if someone did bisect, its only the server being >>>>>> restarted during LOCALIO that could see issues (unlikely thing for >>>>>> someone to be testing for specifically with a bisect). >>>>>> >>>>>>> How do you intend for this to go in? Since most of this is client side, >>>>>>> will this be going in via Trond/Anna's tree? >>>>>> >>>>>> Yes, likely easiest to have it go through Trond/Anna's tree. Trond >>>>>> did have it in his testing tree, maybe your Reviewed-by helps it all >>>>>> land. >>>>>> >>>>>>> You can add: >>>>>>> >>>>>>> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx> >>>>>> >>>>>> Thanks, >>>>>> Mike >>>>>> >>>>> >>>>> Hi all, >>>>> >>>>> These LOCALIO changes didn't land for 6.13 merge, please advise on if >>>>> we might get these changes staged for 6.14 now-ish. >>>> >>>> Works for me. >>>> >>>>> >>>>> Trond and/or Anna, do you feel comfortable picking this series up >>>>> (nfsd cachnges too) or would you like to see any changes before that >>>>> is possible? >>>> >>>> I'll go through the patches once more, but they should be fine. I will need Acked-by's for the NFSD bits from Chuck or Jeff. >>> >>> Looks like Jeff gave his Reviewed-by for the series already. >>> >>> I had asked for some minor changes, haven't seen them go by, >> >> Only thing I was aware of, and addressed in v2, was this: >> https://lore.kernel.org/all/ZzNm0hekxOyAUhib@xxxxxxxxxxxxxxxxxxxxxx/ >> >> I added a module parameters section to the localio.rst and mentioned >> that in v2's 0th header: >> https://lore.kernel.org/r/all/20241114035952.13889-1-snitzer@xxxxxxxxxx/ >> >> with: >> >> "- updated Documentation/filesystems/nfs/localio.rst to reflect the >> percpu_ref change from nfsd_serv to nfsd_net. Also discuss O_DIRECT >> relative to LOCALIO and document the nfs module param (Chuck, I do >> think we need it, otherwise O_DIRECT regressions are possible)." >> >>> but, for the NFSD-related hunks: >>> >>> Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >> >> Thanks! >> >
