2015-07-16 10:02:59

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH] jhash: Deinline jhash, jhash2 and __jhash_nwords

This patch deinlines jhash, jhash2 and __jhash_nwords.

It also removes rhashtable_jhash2(key, length, seed)
because it was merely calling jhash2(key, length, seed).

With this .config: http://busybox.net/~vda/kernel_config,
after deinlining these functions have sizes and callsite counts
as follows:

__jhash_nwords: 72 bytes, 75 calls
jhash: 297 bytes, 111 calls
jhash2: 205 bytes, 136 calls

Total size decrease is about 38,000 bytes:

text data bss dec hex filename
90663567 17221960 36659200 144544727 89d93d7 vmlinux5
90625577 17221864 36659200 144506641 89cff11 vmlinux.after

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Thomas Graf <[email protected]>
CC: Alexander Duyck <[email protected]>
CC: Jozsef Kadlecsik <[email protected]>
CC: Herbert Xu <[email protected]>
CC: [email protected]
---
include/linux/jhash.h | 123 ++------------------------------------------
lib/rhashtable.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 137 insertions(+), 126 deletions(-)

diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index 348c6f4..0b3f55d 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -31,131 +31,14 @@
/* Mask the hash value, i.e (value & jhash_mask(n)) instead of (value % n) */
#define jhash_mask(n) (jhash_size(n)-1)

-/* __jhash_mix -- mix 3 32-bit values reversibly. */
-#define __jhash_mix(a, b, c) \
-{ \
- a -= c; a ^= rol32(c, 4); c += b; \
- b -= a; b ^= rol32(a, 6); a += c; \
- c -= b; c ^= rol32(b, 8); b += a; \
- a -= c; a ^= rol32(c, 16); c += b; \
- b -= a; b ^= rol32(a, 19); a += c; \
- c -= b; c ^= rol32(b, 4); b += a; \
-}
-
-/* __jhash_final - final mixing of 3 32-bit values (a,b,c) into c */
-#define __jhash_final(a, b, c) \
-{ \
- c ^= b; c -= rol32(b, 14); \
- a ^= c; a -= rol32(c, 11); \
- b ^= a; b -= rol32(a, 25); \
- c ^= b; c -= rol32(b, 16); \
- a ^= c; a -= rol32(c, 4); \
- b ^= a; b -= rol32(a, 14); \
- c ^= b; c -= rol32(b, 24); \
-}
-
/* An arbitrary initial parameter */
#define JHASH_INITVAL 0xdeadbeef

-/* jhash - hash an arbitrary key
- * @k: sequence of bytes as key
- * @length: the length of the key
- * @initval: the previous hash, or an arbitray value
- *
- * The generic version, hashes an arbitrary sequence of bytes.
- * No alignment or length assumptions are made about the input key.
- *
- * Returns the hash value of the key. The result depends on endianness.
- */
-static inline u32 jhash(const void *key, u32 length, u32 initval)
-{
- u32 a, b, c;
- const u8 *k = key;
-
- /* Set up the internal state */
- a = b = c = JHASH_INITVAL + length + initval;
-
- /* All but the last block: affect some 32 bits of (a,b,c) */
- while (length > 12) {
- a += __get_unaligned_cpu32(k);
- b += __get_unaligned_cpu32(k + 4);
- c += __get_unaligned_cpu32(k + 8);
- __jhash_mix(a, b, c);
- length -= 12;
- k += 12;
- }
- /* Last block: affect all 32 bits of (c) */
- /* All the case statements fall through */
- switch (length) {
- case 12: c += (u32)k[11]<<24;
- case 11: c += (u32)k[10]<<16;
- case 10: c += (u32)k[9]<<8;
- case 9: c += k[8];
- case 8: b += (u32)k[7]<<24;
- case 7: b += (u32)k[6]<<16;
- case 6: b += (u32)k[5]<<8;
- case 5: b += k[4];
- case 4: a += (u32)k[3]<<24;
- case 3: a += (u32)k[2]<<16;
- case 2: a += (u32)k[1]<<8;
- case 1: a += k[0];
- __jhash_final(a, b, c);
- case 0: /* Nothing left to add */
- break;
- }
-
- return c;
-}
-
-/* jhash2 - hash an array of u32's
- * @k: the key which must be an array of u32's
- * @length: the number of u32's in the key
- * @initval: the previous hash, or an arbitray value
- *
- * Returns the hash value of the key.
- */
-static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
-{
- u32 a, b, c;
-
- /* Set up the internal state */
- a = b = c = JHASH_INITVAL + (length<<2) + initval;
-
- /* Handle most of the key */
- while (length > 3) {
- a += k[0];
- b += k[1];
- c += k[2];
- __jhash_mix(a, b, c);
- length -= 3;
- k += 3;
- }
-
- /* Handle the last 3 u32's: all the case statements fall through */
- switch (length) {
- case 3: c += k[2];
- case 2: b += k[1];
- case 1: a += k[0];
- __jhash_final(a, b, c);
- case 0: /* Nothing left to add */
- break;
- }
-
- return c;
-}
-
+u32 jhash(const void *key, u32 length, u32 initval);
+u32 jhash2(const u32 *k, u32 length, u32 initval);

/* __jhash_nwords - hash exactly 3, 2 or 1 word(s) */
-static inline u32 __jhash_nwords(u32 a, u32 b, u32 c, u32 initval)
-{
- a += initval;
- b += initval;
- c += initval;
-
- __jhash_final(a, b, c);
-
- return c;
-}
+u32 __jhash_nwords(u32 a, u32 b, u32 c, u32 initval);

static inline u32 jhash_3words(u32 a, u32 b, u32 c, u32 initval)
{
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index cc0c697..2492ad4 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -32,6 +32,133 @@
#define HASH_MIN_SIZE 4U
#define BUCKET_LOCKS_PER_CPU 128UL

+
+/* __jhash_mix -- mix 3 32-bit values reversibly. */
+#define __jhash_mix(a, b, c) \
+{ \
+ a -= c; a ^= rol32(c, 4); c += b; \
+ b -= a; b ^= rol32(a, 6); a += c; \
+ c -= b; c ^= rol32(b, 8); b += a; \
+ a -= c; a ^= rol32(c, 16); c += b; \
+ b -= a; b ^= rol32(a, 19); a += c; \
+ c -= b; c ^= rol32(b, 4); b += a; \
+}
+
+/* __jhash_final - final mixing of 3 32-bit values (a,b,c) into c */
+#define __jhash_final(a, b, c) \
+{ \
+ c ^= b; c -= rol32(b, 14); \
+ a ^= c; a -= rol32(c, 11); \
+ b ^= a; b -= rol32(a, 25); \
+ c ^= b; c -= rol32(b, 16); \
+ a ^= c; a -= rol32(c, 4); \
+ b ^= a; b -= rol32(a, 14); \
+ c ^= b; c -= rol32(b, 24); \
+}
+
+/* jhash - hash an arbitrary key
+ * @k: sequence of bytes as key
+ * @length: the length of the key
+ * @initval: the previous hash, or an arbitray value
+ *
+ * The generic version, hashes an arbitrary sequence of bytes.
+ * No alignment or length assumptions are made about the input key.
+ *
+ * Returns the hash value of the key. The result depends on endianness.
+ */
+u32 jhash(const void *key, u32 length, u32 initval)
+{
+ u32 a, b, c;
+ const u8 *k = key;
+
+ /* Set up the internal state */
+ a = b = c = JHASH_INITVAL + length + initval;
+
+ /* All but the last block: affect some 32 bits of (a,b,c) */
+ while (length > 12) {
+ a += __get_unaligned_cpu32(k);
+ b += __get_unaligned_cpu32(k + 4);
+ c += __get_unaligned_cpu32(k + 8);
+ __jhash_mix(a, b, c);
+ length -= 12;
+ k += 12;
+ }
+ /* Last block: affect all 32 bits of (c) */
+ /* All the case statements fall through */
+ switch (length) {
+ case 12: c += (u32)k[11]<<24;
+ case 11: c += (u32)k[10]<<16;
+ case 10: c += (u32)k[9]<<8;
+ case 9: c += k[8];
+ case 8: b += (u32)k[7]<<24;
+ case 7: b += (u32)k[6]<<16;
+ case 6: b += (u32)k[5]<<8;
+ case 5: b += k[4];
+ case 4: a += (u32)k[3]<<24;
+ case 3: a += (u32)k[2]<<16;
+ case 2: a += (u32)k[1]<<8;
+ case 1: a += k[0];
+ __jhash_final(a, b, c);
+ case 0: /* Nothing left to add */
+ break;
+ }
+
+ return c;
+}
+EXPORT_SYMBOL_GPL(jhash);
+
+/* jhash2 - hash an array of u32's
+ * @k: the key which must be an array of u32's
+ * @length: the number of u32's in the key
+ * @initval: the previous hash, or an arbitray value
+ *
+ * Returns the hash value of the key.
+ */
+u32 jhash2(const u32 *k, u32 length, u32 initval)
+{
+ u32 a, b, c;
+
+ /* Set up the internal state */
+ a = b = c = JHASH_INITVAL + (length<<2) + initval;
+
+ /* Handle most of the key */
+ while (length > 3) {
+ a += k[0];
+ b += k[1];
+ c += k[2];
+ __jhash_mix(a, b, c);
+ length -= 3;
+ k += 3;
+ }
+
+ /* Handle the last 3 u32's: all the case statements fall through */
+ switch (length) {
+ case 3: c += k[2];
+ case 2: b += k[1];
+ case 1: a += k[0];
+ __jhash_final(a, b, c);
+ case 0: /* Nothing left to add */
+ break;
+ }
+
+ return c;
+}
+EXPORT_SYMBOL_GPL(jhash2);
+
+/* __jhash_nwords - hash exactly 3, 2 or 1 word(s) */
+u32 __jhash_nwords(u32 a, u32 b, u32 c, u32 initval)
+{
+ a += initval;
+ b += initval;
+ c += initval;
+
+ __jhash_final(a, b, c);
+
+ return c;
+}
+EXPORT_SYMBOL_GPL(__jhash_nwords);
+
+
static u32 head_hashfn(struct rhashtable *ht,
const struct bucket_table *tbl,
const struct rhash_head *he)
@@ -663,11 +790,6 @@ static size_t rounded_hashtable_size(const struct rhashtable_params *params)
(unsigned long)params->min_size);
}

-static u32 rhashtable_jhash2(const void *key, u32 length, u32 seed)
-{
- return jhash2(key, length, seed);
-}
-
/**
* rhashtable_init - initialize a new hash table
* @ht: hash table to be initialized
@@ -773,8 +895,14 @@ int rhashtable_init(struct rhashtable *ht,
ht->p.hashfn = jhash;

if (!(ht->key_len & (sizeof(u32) - 1))) {
+ typedef u32 (*hashfunc)(const void *k, u32 length, u32 initval);
+
ht->key_len /= sizeof(u32);
- ht->p.hashfn = rhashtable_jhash2;
+ /*
+ * jhash2() 1st param is const u32*,
+ * p.hashfn wants const void*. Hence the cast:
+ */
+ ht->p.hashfn = (hashfunc)jhash2;
}
}

--
1.8.1.4


2015-07-16 10:41:51

by Thomas Graf

[permalink] [raw]
Subject: Re: [PATCH] jhash: Deinline jhash, jhash2 and __jhash_nwords

On 07/16/15 at 12:02pm, Denys Vlasenko wrote:
> +/* jhash - hash an arbitrary key
> + * @k: sequence of bytes as key
> + * @length: the length of the key
> + * @initval: the previous hash, or an arbitray value
> + *
> + * The generic version, hashes an arbitrary sequence of bytes.
> + * No alignment or length assumptions are made about the input key.
> + *
> + * Returns the hash value of the key. The result depends on endianness.
> + */
> +u32 jhash(const void *key, u32 length, u32 initval)

Shouldn't these live in lib/jhash.c or something? Otherwise
everyone needs to depend on CONFIG_RHASHTABLE

2015-07-16 12:15:05

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] jhash: Deinline jhash, jhash2 and __jhash_nwords

On 07/16/2015 12:41 PM, Thomas Graf wrote:
> On 07/16/15 at 12:02pm, Denys Vlasenko wrote:
>> +/* jhash - hash an arbitrary key
>> + * @k: sequence of bytes as key
>> + * @length: the length of the key
>> + * @initval: the previous hash, or an arbitray value
>> + *
>> + * The generic version, hashes an arbitrary sequence of bytes.
>> + * No alignment or length assumptions are made about the input key.
>> + *
>> + * Returns the hash value of the key. The result depends on endianness.
>> + */
>> +u32 jhash(const void *key, u32 length, u32 initval)
>
> Shouldn't these live in lib/jhash.c or something? Otherwise
> everyone needs to depend on CONFIG_RHASHTABLE

There is no CONFIG_RHASHTABLE, rhashtable.c is compiled unconditionally.

I will send an alternative patch, which creates jhash.c;
apply whichever version you like most.

2015-07-16 12:29:15

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH] jhash: Deinline jhash, jhash2 and __jhash_nwords

On 07/16/2015 02:15 PM, Denys Vlasenko wrote:
> On 07/16/2015 12:41 PM, Thomas Graf wrote:
>> On 07/16/15 at 12:02pm, Denys Vlasenko wrote:
>>> +/* jhash - hash an arbitrary key
>>> + * @k: sequence of bytes as key
>>> + * @length: the length of the key
>>> + * @initval: the previous hash, or an arbitray value
>>> + *
>>> + * The generic version, hashes an arbitrary sequence of bytes.
>>> + * No alignment or length assumptions are made about the input key.
>>> + *
>>> + * Returns the hash value of the key. The result depends on endianness.
>>> + */
>>> +u32 jhash(const void *key, u32 length, u32 initval)
>>
>> Shouldn't these live in lib/jhash.c or something? Otherwise
>> everyone needs to depend on CONFIG_RHASHTABLE
>
> There is no CONFIG_RHASHTABLE, rhashtable.c is compiled unconditionally.
>
> I will send an alternative patch, which creates jhash.c;
> apply whichever version you like most.

Please also Cc netdev as networking subsys is one of the main users
of jhash in various critical paths. Did you run e.g. some *_RR work
load benchmarks as well to make sure there's no regression?

2015-07-16 14:04:56

by Thomas Graf

[permalink] [raw]
Subject: Re: [PATCH] jhash: Deinline jhash, jhash2 and __jhash_nwords

On 07/16/15 at 02:15pm, Denys Vlasenko wrote:
> On 07/16/2015 12:41 PM, Thomas Graf wrote:
> > On 07/16/15 at 12:02pm, Denys Vlasenko wrote:
> >> +/* jhash - hash an arbitrary key
> >> + * @k: sequence of bytes as key
> >> + * @length: the length of the key
> >> + * @initval: the previous hash, or an arbitray value
> >> + *
> >> + * The generic version, hashes an arbitrary sequence of bytes.
> >> + * No alignment or length assumptions are made about the input key.
> >> + *
> >> + * Returns the hash value of the key. The result depends on endianness.
> >> + */
> >> +u32 jhash(const void *key, u32 length, u32 initval)
> >
> > Shouldn't these live in lib/jhash.c or something? Otherwise
> > everyone needs to depend on CONFIG_RHASHTABLE
>
> There is no CONFIG_RHASHTABLE, rhashtable.c is compiled unconditionally.
>
> I will send an alternative patch, which creates jhash.c;
> apply whichever version you like most.

Right. I misread the CONFIG_TEST_RHASHTABLE. I'm fine with this then
but agree with Daniel that this must be severely tested for
performance regressions.