From: Mike Snitzer <snitzer@kernel.org>
To: linux-nfs@vger.kernel.org
Cc: Jeff Layton <jlayton@kernel.org>,
Chuck Lever <chuck.lever@oracle.com>,
Anna Schumaker <anna@kernel.org>,
Trond Myklebust <trondmy@hammerspace.com>,
NeilBrown <neilb@suse.de>,
linux-fsdevel@vger.kernel.org
Subject: [PATCH v12 21/24] nfs_common: expose localio's required nfsd symbols to nfs client
Date: Mon, 19 Aug 2024 14:17:26 -0400 [thread overview]
Message-ID: <20240819181750.70570-22-snitzer@kernel.org> (raw)
In-Reply-To: <20240819181750.70570-1-snitzer@kernel.org>
From: Mike Snitzer <snitzer@hammerspace.com>
Switch the caching of nfsd_open_local_fh symbol from being within each
nfs_client struct to being globally maintained within nfs_common
(accessible via the global 'nfs_to' nfs_to_nfsd_t struct).
Introduce nfsd_file_file() wrapper that provides access to nfsd_file's
backing file. Keeps nfsd_file structure opaque to NFS client (as
suggested by Jeff Layton).
The addition of nfsd_file_get, nfsd_file_put and nfsd_file_file
symbols prepares for switching the NFS client to using nfsd_file for
localio.
Despite the use of indirect function calls, caching these nfsd symbols
for use by the client offers a ~10% performance win (compared to
always doing get+call+put) for high IOPS workloads.
Suggested-by: Jeff Layton <jlayton@kernel.org> # nfsd_file_file
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
fs/nfs/client.c | 1 -
fs/nfs/localio.c | 17 +++---
fs/nfs_common/nfslocalio.c | 117 +++++++++++++++++++++++++++++++++----
fs/nfsd/filecache.c | 25 ++++++++
fs/nfsd/filecache.h | 1 +
fs/nfsd/localio.c | 2 +-
include/linux/nfs_fs_sb.h | 1 -
include/linux/nfslocalio.h | 27 +++++++--
8 files changed, 163 insertions(+), 28 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 23fe1611cc9f..fe60a82f06d8 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -182,7 +182,6 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
seqlock_init(&clp->cl_boot_lock);
ktime_get_real_ts64(&clp->cl_nfssvc_boot);
clp->cl_rpcclient_localio = ERR_PTR(-EINVAL);
- clp->nfsd_open_local_fh = NULL;
clp->cl_nfssvc_net = NULL;
clp->cl_nfssvc_dom = NULL;
#endif /* CONFIG_NFS_LOCALIO */
diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
index f5ecbd9fefb6..38303427e0b3 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -121,7 +121,7 @@ const struct rpc_program nfslocalio_program = {
*/
static void nfs_local_enable(struct nfs_client *clp, nfs_uuid_t *nfs_uuid)
{
- if (READ_ONCE(clp->nfsd_open_local_fh)) {
+ if (!IS_ERR(clp->cl_rpcclient_localio)) {
set_bit(NFS_CS_LOCAL_IO, &clp->cl_flags);
clp->cl_nfssvc_net = nfs_uuid->net;
clp->cl_nfssvc_dom = nfs_uuid->dom;
@@ -136,8 +136,7 @@ void nfs_local_disable(struct nfs_client *clp)
{
if (test_and_clear_bit(NFS_CS_LOCAL_IO, &clp->cl_flags)) {
trace_nfs_local_disable(clp);
- put_nfsd_open_local_fh();
- clp->nfsd_open_local_fh = NULL;
+ put_nfs_to_nfsd_symbols();
if (!IS_ERR(clp->cl_rpcclient_localio)) {
rpc_shutdown_client(clp->cl_rpcclient_localio);
clp->cl_rpcclient_localio = ERR_PTR(-EINVAL);
@@ -162,16 +161,14 @@ static void nfs_init_localioclient(struct nfs_client *clp)
if (IS_ERR(clp->cl_rpcclient_localio))
goto out;
/* No errors! Assume that localio is supported */
- clp->nfsd_open_local_fh = get_nfsd_open_local_fh();
- if (!clp->nfsd_open_local_fh) {
+ if (!get_nfs_to_nfsd_symbols()) {
rpc_shutdown_client(clp->cl_rpcclient_localio);
clp->cl_rpcclient_localio = ERR_PTR(-EINVAL);
}
out:
- dprintk_rcu("%s: server (%s) %s NFS LOCALIO, nfsd_open_local_fh is %s.\n",
+ dprintk_rcu("%s: server (%s) %s NFS LOCALIO.\n",
__func__, rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR),
- (IS_ERR(clp->cl_rpcclient_localio) ? "does not support" : "supports"),
- (clp->nfsd_open_local_fh ? "set" : "not set"));
+ (IS_ERR(clp->cl_rpcclient_localio) ? "does not support" : "supports"));
}
static bool nfs_server_uuid_is_local(struct nfs_client *clp, nfs_uuid_t *nfs_uuid)
@@ -245,8 +242,8 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
if (mode & ~(FMODE_READ | FMODE_WRITE))
return ERR_PTR(-EINVAL);
- status = clp->nfsd_open_local_fh(clp->cl_nfssvc_net, clp->cl_nfssvc_dom,
- clp->cl_rpcclient, cred, fh, mode, &filp);
+ status = nfs_to.nfsd_open_local_fh(clp->cl_nfssvc_net, clp->cl_nfssvc_dom,
+ clp->cl_rpcclient, cred, fh, mode, &filp);
if (status < 0) {
trace_nfs_local_open_fh(fh, mode, status);
switch (status) {
diff --git a/fs/nfs_common/nfslocalio.c b/fs/nfs_common/nfslocalio.c
index a20ff7607707..087649911b52 100644
--- a/fs/nfs_common/nfslocalio.c
+++ b/fs/nfs_common/nfslocalio.c
@@ -71,27 +71,124 @@ bool nfs_uuid_is_local(const uuid_t *uuid, struct net *net, struct auth_domain *
EXPORT_SYMBOL_GPL(nfs_uuid_is_local);
/*
- * The nfs localio code needs to call into nfsd to do the filehandle -> struct path
- * mapping, but cannot be statically linked, because that will make the nfs module
+ * The nfs localio code needs to call into nfsd using various symbols (below),
+ * but cannot be statically linked, because that will make the nfs module
* depend on the nfsd module.
*
* Instead, do dynamic linking to the nfsd module (via nfs_common module). The
* nfs_common module will only hold a reference on nfsd when localio is in use.
* This allows some sanity checking, like giving up on localio if nfsd isn't loaded.
*/
+DEFINE_MUTEX(nfs_to_nfsd_mutex);
+nfs_to_nfsd_t nfs_to;
+EXPORT_SYMBOL_GPL(nfs_to);
+/* Macro to define nfs_to get and put methods, avoids copy-n-paste bugs */
+#define DEFINE_NFS_TO_NFSD_SYMBOL(NFSD_SYMBOL) \
+static nfs_to_##NFSD_SYMBOL##_t get_##NFSD_SYMBOL(void) \
+{ \
+ return symbol_request(NFSD_SYMBOL); \
+} \
+static void put_##NFSD_SYMBOL(void) \
+{ \
+ symbol_put(NFSD_SYMBOL); \
+ nfs_to.NFSD_SYMBOL = NULL; \
+}
+
+/* The nfs localio code needs to call into nfsd to map filehandle -> struct nfsd_file */
extern int nfsd_open_local_fh(struct net *, struct auth_domain *, struct rpc_clnt *,
- const struct cred *, const struct nfs_fh *,
- const fmode_t, struct file **);
+ const struct cred *, const struct nfs_fh *,
+ const fmode_t, struct file **);
+DEFINE_NFS_TO_NFSD_SYMBOL(nfsd_open_local_fh);
-nfs_to_nfsd_open_t get_nfsd_open_local_fh(void)
+/* The nfs localio code needs to call into nfsd to acquire the nfsd_file */
+extern struct nfsd_file *nfsd_file_get(struct nfsd_file *nf);
+DEFINE_NFS_TO_NFSD_SYMBOL(nfsd_file_get);
+
+/* The nfs localio code needs to call into nfsd to release the nfsd_file */
+extern void nfsd_file_put(struct nfsd_file *nf);
+DEFINE_NFS_TO_NFSD_SYMBOL(nfsd_file_put);
+
+/* The nfs localio code needs to call into nfsd to access the nf->nf_file */
+extern struct file * nfsd_file_file(struct nfsd_file *nf);
+DEFINE_NFS_TO_NFSD_SYMBOL(nfsd_file_file);
+#undef DEFINE_NFS_TO_NFSD_SYMBOL
+
+bool get_nfs_to_nfsd_symbols(void)
{
- return symbol_request(nfsd_open_local_fh);
+ mutex_lock(&nfs_to_nfsd_mutex);
+
+ /* Only get symbols on first reference */
+ if (refcount_read(&nfs_to.ref) == 0)
+ refcount_set(&nfs_to.ref, 1);
+ else {
+ refcount_inc(&nfs_to.ref);
+ mutex_unlock(&nfs_to_nfsd_mutex);
+ return true;
+ }
+
+ nfs_to.nfsd_open_local_fh = get_nfsd_open_local_fh();
+ if (!nfs_to.nfsd_open_local_fh)
+ goto out_nfsd_open_local_fh;
+
+ nfs_to.nfsd_file_get = get_nfsd_file_get();
+ if (!nfs_to.nfsd_file_get)
+ goto out_nfsd_file_get;
+
+ nfs_to.nfsd_file_put = get_nfsd_file_put();
+ if (!nfs_to.nfsd_file_put)
+ goto out_nfsd_file_put;
+
+ nfs_to.nfsd_file_file = get_nfsd_file_file();
+ if (!nfs_to.nfsd_file_file)
+ goto out_nfsd_file_file;
+
+ mutex_unlock(&nfs_to_nfsd_mutex);
+ return true;
+
+out_nfsd_file_file:
+ put_nfsd_file_put();
+out_nfsd_file_put:
+ put_nfsd_file_get();
+out_nfsd_file_get:
+ put_nfsd_open_local_fh();
+out_nfsd_open_local_fh:
+ mutex_unlock(&nfs_to_nfsd_mutex);
+ return false;
}
-EXPORT_SYMBOL_GPL(get_nfsd_open_local_fh);
+EXPORT_SYMBOL_GPL(get_nfs_to_nfsd_symbols);
-void put_nfsd_open_local_fh(void)
+void put_nfs_to_nfsd_symbols(void)
{
- symbol_put(nfsd_open_local_fh);
+ mutex_lock(&nfs_to_nfsd_mutex);
+
+ if (!refcount_dec_and_test(&nfs_to.ref))
+ goto out;
+
+ put_nfsd_open_local_fh();
+ put_nfsd_file_get();
+ put_nfsd_file_put();
+ put_nfsd_file_file();
+out:
+ mutex_unlock(&nfs_to_nfsd_mutex);
+}
+EXPORT_SYMBOL_GPL(put_nfs_to_nfsd_symbols);
+
+static int __init nfslocalio_init(void)
+{
+ refcount_set(&nfs_to.ref, 0);
+
+ nfs_to.nfsd_open_local_fh = NULL;
+ nfs_to.nfsd_file_get = NULL;
+ nfs_to.nfsd_file_put = NULL;
+ nfs_to.nfsd_file_file = NULL;
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(put_nfsd_open_local_fh);
+
+static void __exit nfslocalio_exit(void)
+{
+}
+
+module_init(nfslocalio_init);
+module_exit(nfslocalio_exit);
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 447faa194166..d7c6122231f4 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -39,6 +39,7 @@
#include <linux/fsnotify.h>
#include <linux/seq_file.h>
#include <linux/rhashtable.h>
+#include <linux/nfslocalio.h>
#include "vfs.h"
#include "nfsd.h"
@@ -345,6 +346,10 @@ nfsd_file_get(struct nfsd_file *nf)
return nf;
return NULL;
}
+EXPORT_SYMBOL_GPL(nfsd_file_get);
+
+/* Compile time type checking, not used by anything */
+static nfs_to_nfsd_file_get_t __maybe_unused nfsd_file_get_typecheck = nfsd_file_get;
/**
* nfsd_file_put - put the reference to a nfsd_file
@@ -389,6 +394,26 @@ nfsd_file_put(struct nfsd_file *nf)
if (refcount_dec_and_test(&nf->nf_ref))
nfsd_file_free(nf);
}
+EXPORT_SYMBOL_GPL(nfsd_file_put);
+
+/* Compile time type checking, not used by anything */
+static nfs_to_nfsd_file_put_t __maybe_unused nfsd_file_put_typecheck = nfsd_file_put;
+
+/**
+ * nfsd_file_file - get the backing file of an nfsd_file
+ * @nf: nfsd_file of which to access the backing file.
+ *
+ * Return backing file for @nf.
+ */
+struct file *
+nfsd_file_file(struct nfsd_file *nf)
+{
+ return nf->nf_file;
+}
+EXPORT_SYMBOL_GPL(nfsd_file_file);
+
+/* Compile time type checking, not used by anything */
+static nfs_to_nfsd_file_file_t __maybe_unused nfsd_file_file_typecheck = nfsd_file_file;
static void
nfsd_file_dispose_list(struct list_head *dispose)
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index 6dab41f8541e..ab8a4423edd9 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -56,6 +56,7 @@ int nfsd_file_cache_start_net(struct net *net);
void nfsd_file_cache_shutdown_net(struct net *net);
void nfsd_file_put(struct nfsd_file *nf);
struct nfsd_file *nfsd_file_get(struct nfsd_file *nf);
+struct file *nfsd_file_file(struct nfsd_file *nf);
void nfsd_file_close_inode_sync(struct inode *inode);
void nfsd_file_net_dispose(struct nfsd_net *nn);
bool nfsd_file_is_cached(struct inode *inode);
diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index 9cdea1d1c28a..008b935a3a6c 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -111,7 +111,7 @@ int nfsd_open_local_fh(struct net *cl_nfssvc_net,
EXPORT_SYMBOL_GPL(nfsd_open_local_fh);
/* Compile time type checking, not used by anything */
-static nfs_to_nfsd_open_t __maybe_unused nfsd_open_local_fh_typecheck = nfsd_open_local_fh;
+static nfs_to_nfsd_open_local_fh_t __maybe_unused nfsd_open_local_fh_typecheck = nfsd_open_local_fh;
/*
* UUID_IS_LOCAL XDR functions
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 5edc57657985..10453c6f8ca8 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -134,7 +134,6 @@ struct nfs_client {
struct rpc_clnt * cl_rpcclient_localio;
struct net * cl_nfssvc_net;
struct auth_domain * cl_nfssvc_dom;
- nfs_to_nfsd_open_t nfsd_open_local_fh;
#endif /* CONFIG_NFS_LOCALIO */
};
diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
index c6edcb7c0dcd..6302d36f9112 100644
--- a/include/linux/nfslocalio.h
+++ b/include/linux/nfslocalio.h
@@ -7,6 +7,7 @@
#include <linux/list.h>
#include <linux/uuid.h>
+#include <linux/refcount.h>
#include <linux/sunrpc/clnt.h>
#include <linux/sunrpc/svcauth.h>
#include <linux/nfs.h>
@@ -29,11 +30,27 @@ void nfs_uuid_begin(nfs_uuid_t *);
void nfs_uuid_end(nfs_uuid_t *);
bool nfs_uuid_is_local(const uuid_t *, struct net *, struct auth_domain *);
-typedef int (*nfs_to_nfsd_open_t)(struct net *, struct auth_domain *, struct rpc_clnt *,
- const struct cred *, const struct nfs_fh *,
- const fmode_t, struct file **);
+struct nfsd_file;
-nfs_to_nfsd_open_t get_nfsd_open_local_fh(void);
-void put_nfsd_open_local_fh(void);
+typedef int (*nfs_to_nfsd_open_local_fh_t)(struct net *, struct auth_domain *,
+ struct rpc_clnt *, const struct cred *,
+ const struct nfs_fh *, const fmode_t,
+ struct file **);
+typedef struct nfsd_file * (*nfs_to_nfsd_file_get_t)(struct nfsd_file *);
+typedef void (*nfs_to_nfsd_file_put_t)(struct nfsd_file *);
+typedef struct file * (*nfs_to_nfsd_file_file_t)(struct nfsd_file *);
+
+typedef struct {
+ refcount_t ref;
+ nfs_to_nfsd_open_local_fh_t nfsd_open_local_fh;
+ nfs_to_nfsd_file_get_t nfsd_file_get;
+ nfs_to_nfsd_file_put_t nfsd_file_put;
+ nfs_to_nfsd_file_file_t nfsd_file_file;
+} nfs_to_nfsd_t;
+
+extern nfs_to_nfsd_t nfs_to;
+
+bool get_nfs_to_nfsd_symbols(void);
+void put_nfs_to_nfsd_symbols(void);
#endif /* __LINUX_NFSLOCALIO_H */
--
2.44.0
next prev parent reply other threads:[~2024-08-19 18:18 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-19 18:17 [PATCH v12 00/24] nfs/nfsd: add support for localio Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 01/24] nfs_common: factor out nfs_errtbl and nfs_stat_to_errno Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 02/24] nfs_common: factor out nfs4_errtbl and nfs4_stat_to_errno Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 03/24] nfs: factor out {encode,decode}_opaque_fixed to nfs_xdr.h Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 04/24] nfsd: factor out __fh_verify to allow NULL rqstp to be passed Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 05/24] nfsd: fix nfsfh tracepoints to properly handle NULL rqstp Mike Snitzer
2024-08-21 17:46 ` Jeff Layton
2024-08-21 21:23 ` Mike Snitzer
2024-08-22 15:07 ` Chuck Lever
2024-08-22 16:04 ` Mike Snitzer
2024-08-22 17:07 ` Jeff Layton
2024-08-22 17:20 ` Mike Snitzer
2024-08-22 18:14 ` Chuck Lever III
2024-08-19 18:17 ` [PATCH v12 06/24] nfsd: add nfsd_file_acquire_local() Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 07/24] SUNRPC: remove call_allocate() BUG_ONs Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 08/24] SUNRPC: add rpcauth_map_clnt_to_svc_cred_local Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 09/24] nfs_common: add NFS LOCALIO auxiliary protocol enablement Mike Snitzer
2024-08-21 18:04 ` Jeff Layton
2024-08-21 18:39 ` Jeff Layton
2024-08-19 18:17 ` [PATCH v12 10/24] nfsd: add localio support Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 11/24] nfsd: implement server support for NFS_LOCALIO_PROGRAM Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 12/24] SUNRPC: replace program list with program array Mike Snitzer
2024-08-21 18:31 ` Jeff Layton
2024-08-21 20:40 ` Mike Snitzer
2024-08-21 21:43 ` Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 13/24] nfs: pass struct file to nfs_init_pgio and nfs_init_commit Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 14/24] nfs: add localio support Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 15/24] nfs: enable localio for non-pNFS IO Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 16/24] pnfs/flexfiles: enable localio support Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 17/24] nfs/localio: use dedicated workqueues for filesystem read and write Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 18/24] nfs: implement client support for NFS_LOCALIO_PROGRAM Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 19/24] nfs: add Documentation/filesystems/nfs/localio.rst Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 20/24] nfsd: use GC for nfsd_file returned by nfsd_file_acquire_local Mike Snitzer
2024-08-21 18:34 ` Jeff Layton
2024-08-19 18:17 ` Mike Snitzer [this message]
2024-08-19 18:17 ` [PATCH v12 22/24] nfs: push localio nfsd_file_put call out to client Mike Snitzer
2024-08-21 18:50 ` Jeff Layton
2024-08-19 18:17 ` [PATCH v12 23/24] nfs: switch client to use nfsd_file for localio Mike Snitzer
2024-08-19 18:17 ` [PATCH v12 24/24] nfs: add FAQ section to Documentation/filesystems/nfs/localio.rst Mike Snitzer
2024-08-21 19:03 ` Jeff Layton
2024-08-21 20:12 ` Mike Snitzer
2024-08-21 20:14 ` Mike Snitzer
2024-08-21 23:46 ` Jeff Layton
2024-08-19 18:29 ` [PATCH v12 00/24] nfs/nfsd: add support for localio Chuck Lever III
2024-08-19 18:43 ` Mike Snitzer
2024-08-21 19:20 ` Jeff Layton
2024-08-21 20:05 ` Mike Snitzer
2024-08-22 12:35 ` Jeff Layton
2024-08-22 2:00 ` Mike Snitzer
2024-08-22 12:50 ` Jeff Layton
2024-08-22 15:18 ` Chuck Lever III
2024-08-22 15:42 ` Mike Snitzer
2024-08-21 19:56 ` Chuck Lever
2024-08-21 20:10 ` Mike Snitzer
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=20240819181750.70570-22-snitzer@kernel.org \
--to=snitzer@kernel.org \
--cc=anna@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.