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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).