diff mbox

[net-next,5/7] rhashtable: abstract out function to get hash

Message ID 1473895376-347096-6-git-send-email-tom@herbertland.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Tom Herbert Sept. 14, 2016, 11:22 p.m. UTC
Split out most of rht_key_hashfn which is calculating the hash into
its own function. This way the hash function can be called separately to
get the hash value.

Acked-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/rhashtable.h | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Comments

Herbert Xu Sept. 20, 2016, 9:36 a.m. UTC | #1
Tom Herbert <tom@herbertland.com> wrote:
> Split out most of rht_key_hashfn which is calculating the hash into
> its own function. This way the hash function can be called separately to
> get the hash value.
> 
> Acked-by: Thomas Graf <tgraf@suug.ch>
> Signed-off-by: Tom Herbert <tom@herbertland.com>

I don't get this one.  You're just using jhash, right? Why not
call jhash directly instead of rht_get_key_hashfn?

Cheers,
Thomas Graf Sept. 20, 2016, 5:58 p.m. UTC | #2
On 09/20/16 at 05:36pm, Herbert Xu wrote:
> Tom Herbert <tom@herbertland.com> wrote:
> > Split out most of rht_key_hashfn which is calculating the hash into
> > its own function. This way the hash function can be called separately to
> > get the hash value.
> > 
> > Acked-by: Thomas Graf <tgraf@suug.ch>
> > Signed-off-by: Tom Herbert <tom@herbertland.com>
> 
> I don't get this one.  You're just using jhash, right? Why not
> call jhash directly instead of rht_get_key_hashfn?

FYI, there is a v2 of this series, just so you don't have to do the
work twice.

I understand this particular patch as an effort not to duplicate
hash function selection such as jhash vs jhash2 based on key_len.
Herbert Xu Sept. 21, 2016, 2:46 a.m. UTC | #3
On Tue, Sep 20, 2016 at 07:58:03PM +0200, Thomas Graf wrote:
> 
> I understand this particular patch as an effort not to duplicate
> hash function selection such as jhash vs jhash2 based on key_len.

If the rhashtable params stay non-const as is then this is going
to produce some monstrous code which will be worse than using
jhash unconditionally.

If the rhashtable params are made const then you'll already know
whether jhash or jhash2 is used.

Cheers,
Tom Herbert Sept. 21, 2016, 3:17 a.m. UTC | #4
On Tue, Sep 20, 2016 at 7:46 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Sep 20, 2016 at 07:58:03PM +0200, Thomas Graf wrote:
>>
>> I understand this particular patch as an effort not to duplicate
>> hash function selection such as jhash vs jhash2 based on key_len.
>
> If the rhashtable params stay non-const as is then this is going
> to produce some monstrous code which will be worse than using
> jhash unconditionally.
>
I will look at keep params constant.

Tom

> If the rhashtable params are made const then you'll already know
> whether jhash or jhash2 is used.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff mbox

Patch

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index fd82584..e398a62 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -208,34 +208,42 @@  static inline unsigned int rht_bucket_index(const struct bucket_table *tbl,
 	return (hash >> RHT_HASH_RESERVED_SPACE) & (tbl->size - 1);
 }
 
-static inline unsigned int rht_key_hashfn(
-	struct rhashtable *ht, const struct bucket_table *tbl,
-	const void *key, const struct rhashtable_params params)
+static inline unsigned int rht_key_get_hash(struct rhashtable *ht,
+	const void *key, const struct rhashtable_params params,
+	unsigned int hash_rnd)
 {
 	unsigned int hash;
 
 	/* params must be equal to ht->p if it isn't constant. */
 	if (!__builtin_constant_p(params.key_len))
-		hash = ht->p.hashfn(key, ht->key_len, tbl->hash_rnd);
+		hash = ht->p.hashfn(key, ht->key_len, hash_rnd);
 	else if (params.key_len) {
 		unsigned int key_len = params.key_len;
 
 		if (params.hashfn)
-			hash = params.hashfn(key, key_len, tbl->hash_rnd);
+			hash = params.hashfn(key, key_len, hash_rnd);
 		else if (key_len & (sizeof(u32) - 1))
-			hash = jhash(key, key_len, tbl->hash_rnd);
+			hash = jhash(key, key_len, hash_rnd);
 		else
-			hash = jhash2(key, key_len / sizeof(u32),
-				      tbl->hash_rnd);
+			hash = jhash2(key, key_len / sizeof(u32), hash_rnd);
 	} else {
 		unsigned int key_len = ht->p.key_len;
 
 		if (params.hashfn)
-			hash = params.hashfn(key, key_len, tbl->hash_rnd);
+			hash = params.hashfn(key, key_len, hash_rnd);
 		else
-			hash = jhash(key, key_len, tbl->hash_rnd);
+			hash = jhash(key, key_len, hash_rnd);
 	}
 
+	return hash;
+}
+
+static inline unsigned int rht_key_hashfn(
+	struct rhashtable *ht, const struct bucket_table *tbl,
+	const void *key, const struct rhashtable_params params)
+{
+	unsigned int hash = rht_key_get_hash(ht, key, params, tbl->hash_rnd);
+
 	return rht_bucket_index(tbl, hash);
 }