Re: [for-6.13 PATCH v3 00/14] nfs/nfsd: improvements for LOCALIO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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?

You can add:

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux