Rather than make nfsd_file_do_acquire() more complex (by training
it to conditionally skip both fh_verify() and nfsd_file allocation
and contruction) introduce nfsd_file_acquire_gc_cached() -- which
duplicates the minimalist subset of nfsd_file_do_acquire() needed to
achieve nfsd_file lookup using an opaque @inode_key.
nfsd_file_acquire_gc_cached() only returns a cached and GC'd nfsd_file
obtained using the opaque @inode_key, established from a previous call
to nfsd_file_do_acquire_local() that originally added the GC'd
nfsd_file to the filecache.
Update nfsd_open_local_fh to store @inode_key in @nfs_fh so later
calls can check if it maps to an open GC'd nfsd_file in the filecache
using nfsd_file_acquire_gc_cached(). Its nfsd_file_lookup_locked()
call will only find a match if @cred matches the nfsd_file's nf_cred.
And care is taken to clear the inode_key in nfsd_file_free() if the
nfsd_file has a non-NULL nf_inodep (which is a pointer to the address
of the opaque inode_key stored in the nfs_fh). This avoids any risk
of re-using the old inode_key for a different nfsd_file.
This commit's cached nfsd_file lookup dramatically speeds up LOCALIO
performance, especially for small 4K O_DIRECT IO, e.g.:
before: read: IOPS=376k, BW=1469MiB/s (1541MB/s)(28.7GiB/20001msec)
after: read: IOPS=1037k, BW=4050MiB/s (4247MB/s)(79.1GiB/20002msec)
Note that LOCALIO calls nfsd_open_local_fh() for every IO it issues to
the underlying filesystem using the returned nfsd_file. This is why
caching the opaque 'inode_key' in 'struct nfs_fh' is so helpful for
LOCALIO to quickly return the cached nfsd_file. Whereas regular NFS
avoids fh_verify()'s costly duplicate lookups of the underlying
filesystem's dentry by caching it in 'fh_dentry' of 'struct svc_fh'.
LOCALIO cannot take the same approach, of storing the dentry, without
creating object lifetime issues associated with dentry references
holding server mounts open and preventing unmounts.
Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
---
fs/nfs/inode.c | 3 ++
fs/nfs_common/nfslocalio.c | 2 +-
fs/nfsd/filecache.c | 78 ++++++++++++++++++++++++++++++++++++++
fs/nfsd/filecache.h | 7 ++++
fs/nfsd/localio.c | 46 +++++++++++++++++++---
include/linux/nfs.h | 4 ++
include/linux/nfslocalio.h | 6 +--
7 files changed, 136 insertions(+), 10 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index cc7a32b32676..3051d65e3a89 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2413,6 +2413,9 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
#endif /* CONFIG_NFS_V4 */
#ifdef CONFIG_NFS_V4_2
nfsi->xattr_cache = NULL;
+#endif
+#if IS_ENABLED(CONFIG_NFS_LOCALIO)
+ nfsi->fh.inode_key = NULL;
#endif
nfs_netfs_inode_init(nfsi);
diff --git a/fs/nfs_common/nfslocalio.c b/fs/nfs_common/nfslocalio.c
index 09404d142d1a..bacebaa1e15c 100644
--- a/fs/nfs_common/nfslocalio.c
+++ b/fs/nfs_common/nfslocalio.c
@@ -130,7 +130,7 @@ EXPORT_SYMBOL_GPL(nfs_uuid_invalidate_one_client);
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *uuid,
struct rpc_clnt *rpc_clnt, const struct cred *cred,
- const struct nfs_fh *nfs_fh, const fmode_t fmode)
+ struct nfs_fh *nfs_fh, const fmode_t fmode)
{
struct net *net;
struct nfsd_file *localio;
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 1408166222c5..5ab978ac3555 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -221,6 +221,9 @@ nfsd_file_alloc(struct net *net, struct inode *inode, unsigned char need,
INIT_LIST_HEAD(&nf->nf_gc);
nf->nf_birthtime = ktime_get();
nf->nf_file = NULL;
+#if IS_ENABLED(CONFIG_NFS_LOCALIO)
+ nf->nf_inodep = NULL;
+#endif
nf->nf_cred = get_current_cred();
nf->nf_net = net;
nf->nf_flags = want_gc ?
@@ -285,6 +288,12 @@ nfsd_file_free(struct nfsd_file *nf)
nfsd_file_check_write_error(nf);
nfsd_filp_close(nf->nf_file);
}
+#if IS_ENABLED(CONFIG_NFS_LOCALIO)
+ if (nf->nf_inodep) {
+ *(nf->nf_inodep) = NULL;
+ nf->nf_inodep = NULL;
+ }
+#endif
/*
* If this item is still linked via nf_lru, that's a bug.
@@ -1255,6 +1264,75 @@ nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
return beres;
}
+/**
+ * nfsd_file_acquire_cached - Get cached GC'd open file using inode
+ * @net: The network namespace in which to perform a lookup
+ * @cred: the user credential with which to validate access
+ * @inode_key: inode to use as opaque lookup key
+ * @may_flags: NFSD_MAY_ settings for the file
+ * @pnf: OUT: found cached GC'd "struct nfsd_file" object
+ *
+ * Rather than make nfsd_file_do_acquire more complex (by training
+ * it to conditionally skip fh_verify(), nfsd_file allocation and
+ * contruction) duplicate the minimalist subset of it that is
+ * needed to achieve nfsd_file lookup using the opaque @inode_key.
+ *
+ * The nfsd_file object returned by this API is reference-counted
+ * and garbage-collected. The object is retained for a few
+ * seconds after the final nfsd_file_put() in case the caller
+ * wants to re-use it.
+ *
+ * Return values:
+ * %nfs_ok - @pnf points to an nfsd_file with its reference
+ * count boosted.
+ *
+ * On error, an nfsstat value in network byte order is returned.
+ */
+__be32
+nfsd_file_acquire_cached(struct net *net, const struct cred *cred,
+ void *inode_key, unsigned int may_flags,
+ struct nfsd_file **pnf)
+{
+ unsigned char need = may_flags & NFSD_FILE_MAY_MASK;
+ struct nfsd_file *nf;
+ __be32 status;
+
+ rcu_read_lock();
+ nf = nfsd_file_lookup_locked(net, cred, inode_key, need, true);
+ rcu_read_unlock();
+
+ if (unlikely(!nf))
+ return nfserr_noent;
+
+ /*
+ * If the nf is on the LRU then it holds an extra reference
+ * that must be put if it's removed. It had better not be
+ * the last one however, since we should hold another.
+ */
+ if (nfsd_file_lru_remove(nf))
+ WARN_ON_ONCE(refcount_dec_and_test(&nf->nf_ref));
+
+ if (WARN_ON_ONCE(test_bit(NFSD_FILE_PENDING, &nf->nf_flags) ||
+ !test_bit(NFSD_FILE_HASHED, &nf->nf_flags))) {
+ status = nfserr_inval;
+ goto error;
+ }
+ this_cpu_inc(nfsd_file_cache_hits);
+
+ status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
+ if (status != nfs_ok) {
+error:
+ nfsd_file_put(nf);
+ nf = NULL;
+ } else {
+ this_cpu_inc(nfsd_file_acquisitions);
+ nfsd_file_check_write_error(nf);
+ *pnf = nf;
+ }
+ trace_nfsd_file_acquire(NULL, inode_key, may_flags, nf, status);
+ return status;
+}
+
/**
* nfsd_file_acquire_opened - Get a struct nfsd_file using existing open file
* @rqstp: the RPC transaction being executed
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index cadf3c2689c4..e000f6988dc8 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -47,6 +47,10 @@ struct nfsd_file {
struct list_head nf_gc;
struct rcu_head nf_rcu;
ktime_t nf_birthtime;
+
+#if IS_ENABLED(CONFIG_NFS_LOCALIO)
+ void ** nf_inodep;
+#endif
};
int nfsd_file_cache_init(void);
@@ -71,5 +75,8 @@ __be32 nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
__be32 nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
struct auth_domain *client, struct svc_fh *fhp,
unsigned int may_flags, struct nfsd_file **pnf);
+__be32 nfsd_file_acquire_cached(struct net *net, const struct cred *cred,
+ void *inode_key, unsigned int may_flags,
+ struct nfsd_file **pnf);
int nfsd_file_cache_stats_show(struct seq_file *m, void *v);
#endif /* _FS_NFSD_FILECACHE_H */
diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index f441cb9f74d5..34a229409117 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -58,33 +58,67 @@ void nfsd_localio_ops_init(void)
struct nfsd_file *
nfsd_open_local_fh(struct net *net, struct auth_domain *dom,
struct rpc_clnt *rpc_clnt, const struct cred *cred,
- const struct nfs_fh *nfs_fh, const fmode_t fmode)
+ struct nfs_fh *nfs_fh, const fmode_t fmode)
{
int mayflags = NFSD_MAY_LOCALIO;
struct svc_cred rq_cred;
struct svc_fh fh;
struct nfsd_file *localio;
+ void *nf_inode_key;
__be32 beres;
if (nfs_fh->size > NFS4_FHSIZE)
return ERR_PTR(-EINVAL);
- /* nfs_fh -> svc_fh */
- fh_init(&fh, NFS4_FHSIZE);
- fh.fh_handle.fh_size = nfs_fh->size;
- memcpy(fh.fh_handle.fh_raw, nfs_fh->data, nfs_fh->size);
-
if (fmode & FMODE_READ)
mayflags |= NFSD_MAY_READ;
if (fmode & FMODE_WRITE)
mayflags |= NFSD_MAY_WRITE;
+ /*
+ * Check if 'inode_key' stored in @nfs_fh maps to an
+ * open nfsd_file in the filecache (from a previous
+ * nfsd_open_local_fh).
+ *
+ * The 'inode_key' may have become stale (due to nfsd_file
+ * being free'd by filecache GC) so the lookup will fail
+ * gracefully by falling back to using nfsd_file_acquire_local().
+ */
+ nf_inode_key = READ_ONCE(nfs_fh->inode_key);
+ if (nf_inode_key) {
+ beres = nfsd_file_acquire_cached(net, cred,
+ nf_inode_key,
+ mayflags, &localio);
+ if (beres == nfs_ok)
+ return localio;
+ /*
+ * reset stale nfs_fh->inode_key and fallthru
+ * to use normal nfsd_file_acquire_local().
+ */
+ nfs_fh->inode_key = NULL;
+ }
+
+ /* nfs_fh -> svc_fh */
+ fh_init(&fh, NFS4_FHSIZE);
+ fh.fh_handle.fh_size = nfs_fh->size;
+ memcpy(fh.fh_handle.fh_raw, nfs_fh->data, nfs_fh->size);
+
svcauth_map_clnt_to_svc_cred_local(rpc_clnt, cred, &rq_cred);
beres = nfsd_file_acquire_local(net, &rq_cred, dom,
&fh, mayflags, &localio);
if (beres)
localio = ERR_PTR(nfs_stat_to_errno(be32_to_cpu(beres)));
+ else {
+ /*
+ * opaque 'inode_key' has a 1:1 mapping to both an
+ * nfsd_file and nfs_fh struct (And the nfs_fh is shared
+ * by all NFS client threads. So there is no risk of
+ * storing competing addresses in nfsd_file->nf_inodep
+ */
+ localio->nf_inodep = (void **) &nfs_fh->inode_key;
+ nfs_fh->inode_key = localio->nf_inode;
+ }
fh_put(&fh);
if (rq_cred.cr_group_info)
diff --git a/include/linux/nfs.h b/include/linux/nfs.h
index 9ad727ddfedb..56c894575c70 100644
--- a/include/linux/nfs.h
+++ b/include/linux/nfs.h
@@ -29,6 +29,10 @@
struct nfs_fh {
unsigned short size;
unsigned char data[NFS_MAXFHSIZE];
+#if IS_ENABLED(CONFIG_NFS_LOCALIO)
+ /* 'inode_key' is an opaque key used for nfsd_file hash lookups */
+ void * inode_key;
+#endif
};
/*
diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
index 3982fea79919..619eb1961ed6 100644
--- a/include/linux/nfslocalio.h
+++ b/include/linux/nfslocalio.h
@@ -43,7 +43,7 @@ void nfs_uuid_invalidate_one_client(nfs_uuid_t *nfs_uuid);
/* localio needs to map filehandle -> struct nfsd_file */
extern struct nfsd_file *
nfsd_open_local_fh(struct net *, struct auth_domain *, struct rpc_clnt *,
- const struct cred *, const struct nfs_fh *,
+ const struct cred *, struct nfs_fh *,
const fmode_t) __must_hold(rcu);
struct nfsd_localio_operations {
@@ -53,7 +53,7 @@ struct nfsd_localio_operations {
struct auth_domain *,
struct rpc_clnt *,
const struct cred *,
- const struct nfs_fh *,
+ struct nfs_fh *,
const fmode_t);
void (*nfsd_file_put_local)(struct nfsd_file *);
struct file *(*nfsd_file_file)(struct nfsd_file *);
@@ -64,7 +64,7 @@ extern const struct nfsd_localio_operations *nfs_to;
struct nfsd_file *nfs_open_local_fh(nfs_uuid_t *,
struct rpc_clnt *, const struct cred *,
- const struct nfs_fh *, const fmode_t);
+ struct nfs_fh *, const fmode_t);
static inline void nfs_to_nfsd_file_put_local(struct nfsd_file *localio)
{
--
2.44.0
