public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: linux-nfs@vger.kernel.org
Cc: Anna Schumaker <anna@kernel.org>,
	Trond Myklebust <trondmy@hammerspace.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>, NeilBrown <neilb@suse.de>
Subject: [for-6.13 PATCH v2 15/15] nfs: probe for LOCALIO when v3 client reconnects to server
Date: Wed, 13 Nov 2024 22:59:52 -0500	[thread overview]
Message-ID: <20241114035952.13889-16-snitzer@kernel.org> (raw)
In-Reply-To: <20241114035952.13889-1-snitzer@kernel.org>

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().

This approach works for most use-cases but it doesn't handle the
possibility of using HA to migrate an NFS server local to the same
host as an NFS client that is actively connected to the migrated NFS
server. NFSv3 LOCALIO won't handle this case given it relies on the
client having been flagged with NFS_CS_LOCAL_IO when the client was
created on the host (e.g. mount time).

Alternatve approaches have been discussed but there isn't clear
consensus yet.

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/internal.h          |  5 +++++
 fs/nfs/localio.c           | 18 +++++++++++++++++-
 fs/nfs/nfs3proc.c          | 32 +++++++++++++++++++++++++++++---
 fs/nfs_common/nfslocalio.c |  1 +
 include/linux/nfslocalio.h |  3 ++-
 5 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index ad9c56bc977bf..14f4d871b6a30 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_probe(struct nfs_client *clp) {}
@@ -498,6 +499,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 1eee5aac28843..d5152aa46813c 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -65,6 +65,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, &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
  */
@@ -187,8 +198,13 @@ void nfs_local_probe(struct nfs_client *clp)
 
 	if (!nfs_uuid_begin(&clp->cl_uuid))
 		return;
-	if (nfs_server_uuid_is_local(clp))
+	if (nfs_server_uuid_is_local(clp)) {
 		nfs_localio_enable_client(clp);
+		/* Set hint that client and server are LOCALIO capable */
+		spin_lock(&clp->cl_uuid.lock);
+		set_bit(NFS_CS_LOCAL_IO, &clp->cl_flags);
+		spin_unlock(&clp->cl_uuid.lock);
+	}
 	nfs_uuid_end(&clp->cl_uuid);
 }
 EXPORT_SYMBOL_GPL(nfs_local_probe);
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 1566163c6d85b..6ed2e4466002d 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -844,6 +844,27 @@ 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.
+	 */
+	/* Arbitrary throttle to reduce nfs_local_probe_async() frequency */
+	if ((nfs_uuid->local_probe_count++ & 511) == 0) {
+		if (unlikely(!nfs_server_is_local(clp) && nfs_server_was_local(clp)))
+			nfs_local_probe_async(clp);
+	}
+#endif
+}
+
 static int nfs3_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)
 {
 	struct inode *inode = hdr->inode;
@@ -855,8 +876,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 +910,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 0a26c0ca99c21..41c0077ead721 100644
--- a/fs/nfs_common/nfslocalio.c
+++ b/fs/nfs_common/nfslocalio.c
@@ -43,6 +43,7 @@ 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);
+	nfs_uuid->local_probe_count = 0;
 }
 EXPORT_SYMBOL_GPL(nfs_uuid_init);
 
diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
index c68a529230c14..e05f1cf3cf476 100644
--- a/include/linux/nfslocalio.h
+++ b/include/linux/nfslocalio.h
@@ -27,7 +27,8 @@ struct nfs_file_localio;
  */
 typedef struct {
 	uuid_t uuid;
-	/* sadly this struct is just over a cacheline, avoid bouncing */
+	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


  parent reply	other threads:[~2024-11-14  4:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-14  3:59 [for-6.13 PATCH v2 00/15] nfs/nfsd: fixes and improvements for LOCALIO Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 01/15] nfs_common: must not hold RCU while calling nfsd_file_put_local Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 02/15] nfs/localio: add direct IO enablement with sync and async IO support Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 03/15] nfsd: add nfsd_file_{get,put} to 'nfs_to' nfsd_localio_operations Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 04/15] nfs_common: rename functions that invalidate LOCALIO nfs_clients Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 05/15] nfs_common: move localio_lock to new lock member of nfs_uuid_t Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 06/15] nfs: cache all open LOCALIO nfsd_file(s) in client Mike Snitzer
2024-11-15  3:12   ` NeilBrown
2024-11-15 16:49     ` Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 07/15] nfsd: update percpu_ref to manage references on nfsd_net Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 08/15] nfsd: rename nfsd_serv_ prefixed methods and variables with nfsd_net_ Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 09/15] nfsd: nfsd_file_acquire_local no longer returns GC'd nfsd_file Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 10/15] nfs_common: rename nfslocalio nfs_uuid_lock to nfs_uuids_lock Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 11/15] nfs_common: track all open nfsd_files per LOCALIO nfs_client Mike Snitzer
2024-11-14 18:53   ` Jeff Layton
2024-11-14 23:22     ` Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 12/15] nfs_common: add nfs_localio trace events Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 13/15] nfs/localio: remove redundant code and simplify LOCALIO enablement Mike Snitzer
2024-11-14 15:31   ` [for-6.13 PATCH v2-fixed " Mike Snitzer
2024-11-14  3:59 ` [for-6.13 PATCH v2 14/15] nfs: probe for LOCALIO when v4 client reconnects to server Mike Snitzer
2024-11-14  3:59 ` Mike Snitzer [this message]
2024-11-15  3:55   ` [for-6.13 PATCH v2 15/15] nfs: probe for LOCALIO when v3 " NeilBrown
2024-11-14 16:14 ` (subset) [for-6.13 PATCH v2 00/15] nfs/nfsd: fixes and improvements for LOCALIO cel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241114035952.13889-16-snitzer@kernel.org \
    --to=snitzer@kernel.org \
    --cc=anna@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=trondmy@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox