On Sat, 09 Nov 2024, Mike Snitzer wrote:
> Re-enabling NFSv3 LOCALIO is made more complex (than NFSv4) because v3
> is stateless. As such, the hueristic used to identify a LOCALIO probe
> point is more adhoc by nature: if/when NFSv3 client IO begins to
> complete again in terms of normal RPC-based NFSv3 server IO, attempt
> nfs_local_probe_async().
>
> Care is taken to throttle the frequency of nfs_local_probe_async(),
> otherwise there could be a flood of repeat calls to
> nfs_local_probe_async().
I think it would be good to limit this to only probing when the network
connection is reestablished - assuming we can ignore connectionless
protocols like UDP.
I think you can stash rpc_clnt->cl_xprt->connect_cookie and check if
that has changed or not. If not, then there is no point probing again.
Thanks,
NeilBrown
>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfs/internal.h | 5 +++++
> fs/nfs/localio.c | 11 +++++++++++
> fs/nfs/nfs3proc.c | 34 +++++++++++++++++++++++++++++++---
> fs/nfs_common/nfslocalio.c | 4 ++++
> include/linux/nfs_fs_sb.h | 1 +
> include/linux/nfslocalio.h | 4 +++-
> 6 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index efd42efd9405..fb1ab7cee6b9 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -470,6 +470,7 @@ extern int nfs_local_commit(struct nfsd_file *,
> struct nfs_commit_data *,
> const struct rpc_call_ops *, int);
> extern bool nfs_server_is_local(const struct nfs_client *clp);
> +extern bool nfs_server_was_local(const struct nfs_client *clp);
>
> #else /* CONFIG_NFS_LOCALIO */
> static inline void nfs_local_disable(struct nfs_client *clp) {}
> @@ -499,6 +500,10 @@ static inline bool nfs_server_is_local(const struct nfs_client *clp)
> {
> return false;
> }
> +static inline bool nfs_server_was_local(const struct nfs_client *clp)
> +{
> + return false;
> +}
> #endif /* CONFIG_NFS_LOCALIO */
>
> /* super.c */
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index 710e537b3402..1559dc2f1850 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -64,6 +64,17 @@ bool nfs_server_is_local(const struct nfs_client *clp)
> }
> EXPORT_SYMBOL_GPL(nfs_server_is_local);
>
> +static inline bool nfs_client_was_local(const struct nfs_client *clp)
> +{
> + return !!test_bit(NFS_CS_LOCAL_IO_CAPABLE, &clp->cl_flags);
> +}
> +
> +bool nfs_server_was_local(const struct nfs_client *clp)
> +{
> + return nfs_client_was_local(clp) && localio_enabled;
> +}
> +EXPORT_SYMBOL_GPL(nfs_server_was_local);
> +
> /*
> * UUID_IS_LOCAL XDR functions
> */
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index 1566163c6d85..4d2018760e9b 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -844,6 +844,29 @@ nfs3_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
> return status;
> }
>
> +static void nfs3_local_probe(struct nfs_server *server)
> +{
> +#if IS_ENABLED(CONFIG_NFS_LOCALIO)
> + struct nfs_client *clp = server->nfs_client;
> + nfs_uuid_t *nfs_uuid = &clp->cl_uuid;
> +
> + if (likely(!nfs_server_was_local(clp)))
> + return;
> + /*
> + * Try re-enabling LOCALIO if it was previously enabled, but
> + * was disabled due to server restart, and IO has successfully
> + * completed in terms of normal RPC.
> + */
> + mutex_lock(&nfs_uuid->local_probe_mutex);
> + /* Arbitrary throttle to reduce nfs_local_probe_async() frequency */
> + if ((nfs_uuid->local_probe_count++ & 255) == 0) {
> + if (unlikely(!nfs_server_is_local(clp) && nfs_server_was_local(clp)))
> + nfs_local_probe_async(clp);
> + }
> + mutex_unlock(&nfs_uuid->local_probe_mutex);
> +#endif
> +}
> +
> static int nfs3_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
> {
> struct inode *inode = hdr->inode;
> @@ -855,8 +878,11 @@ static int nfs3_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
> if (nfs3_async_handle_jukebox(task, inode))
> return -EAGAIN;
>
> - if (task->tk_status >= 0 && !server->read_hdrsize)
> - cmpxchg(&server->read_hdrsize, 0, hdr->res.replen);
> + if (task->tk_status >= 0) {
> + if (!server->read_hdrsize)
> + cmpxchg(&server->read_hdrsize, 0, hdr->res.replen);
> + nfs3_local_probe(server);
> + }
>
> nfs_invalidate_atime(inode);
> nfs_refresh_inode(inode, &hdr->fattr);
> @@ -886,8 +912,10 @@ static int nfs3_write_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
>
> if (nfs3_async_handle_jukebox(task, inode))
> return -EAGAIN;
> - if (task->tk_status >= 0)
> + if (task->tk_status >= 0) {
> nfs_writeback_update_inode(hdr);
> + nfs3_local_probe(NFS_SERVER(inode));
> + }
> return 0;
> }
>
> diff --git a/fs/nfs_common/nfslocalio.c b/fs/nfs_common/nfslocalio.c
> index fb376d38ac9a..852ba8fd73f3 100644
> --- a/fs/nfs_common/nfslocalio.c
> +++ b/fs/nfs_common/nfslocalio.c
> @@ -43,6 +43,8 @@ void nfs_uuid_init(nfs_uuid_t *nfs_uuid)
> INIT_LIST_HEAD(&nfs_uuid->list);
> INIT_LIST_HEAD(&nfs_uuid->files);
> spin_lock_init(&nfs_uuid->lock);
> + mutex_init(&nfs_uuid->local_probe_mutex);
> + nfs_uuid->local_probe_count = 0;
> }
> EXPORT_SYMBOL_GPL(nfs_uuid_init);
>
> @@ -143,6 +145,8 @@ void nfs_localio_enable_client(struct nfs_client *clp)
>
> spin_lock(&nfs_uuid->lock);
> set_bit(NFS_CS_LOCAL_IO, &clp->cl_flags);
> + /* Also set hint that client and server are LOCALIO capable */
> + set_bit(NFS_CS_LOCAL_IO_CAPABLE, &clp->cl_flags);
> trace_nfs_localio_enable_client(clp);
> spin_unlock(&nfs_uuid->lock);
> }
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 63d7e0f478d8..45906c402c98 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -51,6 +51,7 @@ struct nfs_client {
> #define NFS_CS_REUSEPORT 8 /* - reuse src port on reconnect */
> #define NFS_CS_PNFS 9 /* - Server used for pnfs */
> #define NFS_CS_LOCAL_IO 10 /* - client is local */
> +#define NFS_CS_LOCAL_IO_CAPABLE 11 /* - client was previously local */
> struct sockaddr_storage cl_addr; /* server identifier */
> size_t cl_addrlen;
> char * cl_hostname; /* hostname of server */
> diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
> index c68a529230c1..3dfef0bb18fe 100644
> --- a/include/linux/nfslocalio.h
> +++ b/include/linux/nfslocalio.h
> @@ -27,7 +27,9 @@ struct nfs_file_localio;
> */
> typedef struct {
> uuid_t uuid;
> - /* sadly this struct is just over a cacheline, avoid bouncing */
> + struct mutex local_probe_mutex;
> + unsigned local_probe_count;
> + /* sadly this struct is over a cacheline, avoid bouncing */
> spinlock_t ____cacheline_aligned lock;
> struct list_head list;
> spinlock_t *list_lock; /* nn->local_clients_lock */
> --
> 2.44.0
>
>
>
