diff mbox series

[RFC,28/30] NFSD: Set up an rhashtable for the filecache

Message ID 165590735022.75778.7652622979487182880.stgit@manet.1015granger.net (mailing list archive)
State Superseded
Headers show
Series Overhaul NFSD filecache | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Chuck Lever III June 22, 2022, 2:15 p.m. UTC
Add code to initialize and tear down an rhashtable. The rhashtable
is not used yet.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/filecache.c |  131 +++++++++++++++++++++++++++++++++++++++++++--------
 fs/nfsd/filecache.h |    1 
 2 files changed, 111 insertions(+), 21 deletions(-)

Comments

Al Viro June 23, 2022, 10:56 p.m. UTC | #1
On Wed, Jun 22, 2022 at 10:15:50AM -0400, Chuck Lever wrote:

> +static u32 nfsd_file_obj_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	const struct nfsd_file *nf = data;
> +
> +	return jhash2((const u32 *)&nf->nf_inode,
> +		      sizeof_field(struct nfsd_file, nf_inode) / sizeof(u32),
> +		      seed);

Out of curiosity - what are you using to allocate those?  Because if
it's a slab, then middle bits of address (i.e. lower bits of
(unsigned long)data / L1_CACHE_BYTES) would better be random enough...
Chuck Lever III June 23, 2022, 11:51 p.m. UTC | #2
> On Jun 23, 2022, at 6:56 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 
> On Wed, Jun 22, 2022 at 10:15:50AM -0400, Chuck Lever wrote:
> 
>> +static u32 nfsd_file_obj_hashfn(const void *data, u32 len, u32 seed)
>> +{
>> +	const struct nfsd_file *nf = data;
>> +
>> +	return jhash2((const u32 *)&nf->nf_inode,
>> +		      sizeof_field(struct nfsd_file, nf_inode) / sizeof(u32),
>> +		      seed);
> 
> Out of curiosity - what are you using to allocate those?  Because if
> it's a slab, then middle bits of address (i.e. lower bits of
> (unsigned long)data / L1_CACHE_BYTES) would better be random enough...

 261 static struct nfsd_file *
 262 nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
 263 {
 264         static atomic_t nfsd_file_id;
 265         struct nfsd_file *nf;
 266 
 267         nf = kmem_cache_alloc(nfsd_file_slab, GFP_KERNEL);

Was wondering about that. pahole says struct nfsd_file is 112
bytes on my system.


--
Chuck Lever
Chuck Lever III June 24, 2022, 12:14 a.m. UTC | #3
> On Jun 23, 2022, at 7:51 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
>> On Jun 23, 2022, at 6:56 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> 
>> On Wed, Jun 22, 2022 at 10:15:50AM -0400, Chuck Lever wrote:
>> 
>>> +static u32 nfsd_file_obj_hashfn(const void *data, u32 len, u32 seed)
>>> +{
>>> +	const struct nfsd_file *nf = data;
>>> +
>>> +	return jhash2((const u32 *)&nf->nf_inode,
>>> +		      sizeof_field(struct nfsd_file, nf_inode) / sizeof(u32),
>>> +		      seed);
>> 
>> Out of curiosity - what are you using to allocate those?  Because if
>> it's a slab, then middle bits of address (i.e. lower bits of
>> (unsigned long)data / L1_CACHE_BYTES) would better be random enough...
> 
> 261 static struct nfsd_file *
> 262 nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
> 263 {
> 264         static atomic_t nfsd_file_id;
> 265         struct nfsd_file *nf;
> 266 
> 267         nf = kmem_cache_alloc(nfsd_file_slab, GFP_KERNEL);
> 
> Was wondering about that. pahole says struct nfsd_file is 112
> bytes on my system.

Oops. nfsd_file_obj_hashfn() is supposed to be generating the
hash value based on the address stored in the nf_inode field.
So it's an inode pointer, alloced via kmem_cache_alloc by default.


--
Chuck Lever
Al Viro June 24, 2022, 12:29 a.m. UTC | #4
On Fri, Jun 24, 2022 at 12:14:53AM +0000, Chuck Lever III wrote:
> 
> > On Jun 23, 2022, at 7:51 PM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> > 
> >> On Jun 23, 2022, at 6:56 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >> 
> >> On Wed, Jun 22, 2022 at 10:15:50AM -0400, Chuck Lever wrote:
> >> 
> >>> +static u32 nfsd_file_obj_hashfn(const void *data, u32 len, u32 seed)
> >>> +{
> >>> +	const struct nfsd_file *nf = data;
> >>> +
> >>> +	return jhash2((const u32 *)&nf->nf_inode,
> >>> +		      sizeof_field(struct nfsd_file, nf_inode) / sizeof(u32),
> >>> +		      seed);
> >> 
> >> Out of curiosity - what are you using to allocate those?  Because if
> >> it's a slab, then middle bits of address (i.e. lower bits of
> >> (unsigned long)data / L1_CACHE_BYTES) would better be random enough...
> > 
> > 261 static struct nfsd_file *
> > 262 nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
> > 263 {
> > 264         static atomic_t nfsd_file_id;
> > 265         struct nfsd_file *nf;
> > 266 
> > 267         nf = kmem_cache_alloc(nfsd_file_slab, GFP_KERNEL);
> > 
> > Was wondering about that. pahole says struct nfsd_file is 112
> > bytes on my system.
> 
> Oops. nfsd_file_obj_hashfn() is supposed to be generating the
> hash value based on the address stored in the nf_inode field.
> So it's an inode pointer, alloced via kmem_cache_alloc by default.

inode pointers are definitely "divide by L1_CACHE_BYTES and take lower
bits" fodder...
diff mbox series

Patch

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 75cb1f52152c..a491519598fc 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -13,6 +13,7 @@ 
 #include <linux/fsnotify_backend.h>
 #include <linux/fsnotify.h>
 #include <linux/seq_file.h>
+#include <linux/rhashtable.h>
 
 #include "vfs.h"
 #include "nfsd.h"
@@ -65,6 +66,107 @@  static atomic_long_t			nfsd_filecache_count;
 static atomic_long_t			nfsd_file_total_age;
 static struct delayed_work		nfsd_filecache_laundrette;
 
+static struct rhashtable nfsd_file_rhash_tbl ____cacheline_aligned_in_smp;
+
+struct nfsd_file_lookup_key {
+	struct inode *inode;
+	struct net *net;
+	const struct cred *cred;
+	unsigned char type;
+	unsigned char need;
+};
+
+enum {
+	NFSD_FILE_KEY_INODE,
+	NFSD_FILE_KEY_FULL,
+};
+
+static bool
+nfsd_match_cred(const struct cred *c1, const struct cred *c2)
+{
+	int i;
+
+	if (!uid_eq(c1->fsuid, c2->fsuid))
+		return false;
+	if (!gid_eq(c1->fsgid, c2->fsgid))
+		return false;
+	if (c1->group_info == NULL || c2->group_info == NULL)
+		return c1->group_info == c2->group_info;
+	if (c1->group_info->ngroups != c2->group_info->ngroups)
+		return false;
+	for (i = 0; i < c1->group_info->ngroups; i++) {
+		if (!gid_eq(c1->group_info->gid[i], c2->group_info->gid[i]))
+			return false;
+	}
+	return true;
+}
+
+/**
+ * nfsd_file_obj_hashfn - Compute the hash value of an nfsd_file
+ * @data: object on which to compute the hash value
+ * @len: rhash table's key_len parameter (unused)
+ * @seed: rhash table's random seed of the day
+ *
+ * Return value:
+ *   Computed 32-bit hash value
+ */
+static u32 nfsd_file_obj_hashfn(const void *data, u32 len, u32 seed)
+{
+	const struct nfsd_file *nf = data;
+
+	return jhash2((const u32 *)&nf->nf_inode,
+		      sizeof_field(struct nfsd_file, nf_inode) / sizeof(u32),
+		      seed);
+}
+
+/**
+ * nfsd_file_obj_cmpfn - Match a cache item against search criteria
+ * @arg: search criteria
+ * @ptr: cache item to check
+ *
+ * Return values:
+ *   %0 - Item matches search criteria
+ *   %1 - Item does not match search criteria
+ */
+static int nfsd_file_obj_cmpfn(struct rhashtable_compare_arg *arg,
+			       const void *ptr)
+{
+	const struct nfsd_file_lookup_key *key = arg->key;
+	const struct nfsd_file *nf = ptr;
+
+	switch (key->type) {
+	case NFSD_FILE_KEY_INODE:
+		if (nf->nf_inode != key->inode)
+			return 1;
+		break;
+	case NFSD_FILE_KEY_FULL:
+		if (nf->nf_inode != key->inode)
+			return 1;
+		if (nf->nf_may != key->need)
+			return 1;
+		if (nf->nf_net != key->net)
+			return 1;
+		if (!nfsd_match_cred(nf->nf_cred, key->cred))
+			return 1;
+		if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags))
+			return 1;
+		break;
+	}
+
+	return 0;
+}
+
+static const struct rhashtable_params nfsd_file_rhash_params = {
+	.key_len		= sizeof_field(struct nfsd_file, nf_inode),
+	.key_offset		= offsetof(struct nfsd_file, nf_inode),
+	.head_offset		= offsetof(struct nfsd_file, nf_rhash),
+	.obj_hashfn		= nfsd_file_obj_hashfn,
+	.obj_cmpfn		= nfsd_file_obj_cmpfn,
+	.max_size		= 131072,	/* buckets */
+	.min_size		= 1024,		/* buckets */
+	.automatic_shrinking	= true,
+};
+
 static void
 nfsd_file_schedule_laundrette(void)
 {
@@ -697,13 +799,18 @@  static const struct fsnotify_ops nfsd_file_fsnotify_ops = {
 int
 nfsd_file_cache_init(void)
 {
-	int		ret = -ENOMEM;
+	int		ret;
 	unsigned int	i;
 
 	lockdep_assert_held(&nfsd_mutex);
 	if (test_and_set_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags) == 1)
 		return 0;
 
+	ret = rhashtable_init(&nfsd_file_rhash_tbl, &nfsd_file_rhash_params);
+	if (ret)
+		return ret;
+
+	ret = -ENOMEM;
 	nfsd_filecache_wq = alloc_workqueue("nfsd_filecache", 0, 0);
 	if (!nfsd_filecache_wq)
 		goto out;
@@ -781,6 +888,7 @@  nfsd_file_cache_init(void)
 	nfsd_file_hashtbl = NULL;
 	destroy_workqueue(nfsd_filecache_wq);
 	nfsd_filecache_wq = NULL;
+	rhashtable_destroy(&nfsd_file_rhash_tbl);
 	goto out;
 }
 
@@ -904,6 +1012,7 @@  nfsd_file_cache_shutdown(void)
 	nfsd_file_hashtbl = NULL;
 	destroy_workqueue(nfsd_filecache_wq);
 	nfsd_filecache_wq = NULL;
+	rhashtable_destroy(&nfsd_file_rhash_tbl);
 
 	for_each_possible_cpu(i) {
 		this_cpu_write(nfsd_file_cache_hits, 0);
@@ -915,26 +1024,6 @@  nfsd_file_cache_shutdown(void)
 	}
 }
 
-static bool
-nfsd_match_cred(const struct cred *c1, const struct cred *c2)
-{
-	int i;
-
-	if (!uid_eq(c1->fsuid, c2->fsuid))
-		return false;
-	if (!gid_eq(c1->fsgid, c2->fsgid))
-		return false;
-	if (c1->group_info == NULL || c2->group_info == NULL)
-		return c1->group_info == c2->group_info;
-	if (c1->group_info->ngroups != c2->group_info->ngroups)
-		return false;
-	for (i = 0; i < c1->group_info->ngroups; i++) {
-		if (!gid_eq(c1->group_info->gid[i], c2->group_info->gid[i]))
-			return false;
-	}
-	return true;
-}
-
 static struct nfsd_file *
 nfsd_file_find_locked(struct inode *inode, unsigned int may_flags,
 			unsigned int hashval, struct net *net)
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index 31dc65f82c75..7fc017e7b09e 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -29,6 +29,7 @@  struct nfsd_file_mark {
  * never be dereferenced, only used for comparison.
  */
 struct nfsd_file {
+	struct rhash_head	nf_rhash;
 	struct hlist_node	nf_node;
 	struct list_head	nf_lru;
 	struct rcu_head		nf_rcu;