diff mbox series

[for-6.13,19/19] nfs: probe for LOCALIO when v3 client reconnects to server

Message ID 20241108234002.16392-20-snitzer@kernel.org (mailing list archive)
State New
Headers show
Series nfs/nfsd: fixes and improvements for LOCALIO | expand

Commit Message

Mike Snitzer Nov. 8, 2024, 11:40 p.m. UTC 
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().

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 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(-)

Comments

NeilBrown Nov. 11, 2024, 3:06 a.m. UTC | #1 
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@kernel.org>
> ---
>  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
> 
> 
>
diff mbox series

Patch

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 */