[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALx6S367jArRC3eKSKsv7gFrgPxyDHWdtvfeO1SmQR91eYxwSw@mail.gmail.com>
Date: Thu, 16 Jul 2015 08:43:25 -0700
From: Tom Herbert <tom@...bertland.com>
To: Denys Vlasenko <dvlasenk@...hat.com>
Cc: Thomas Graf <tgraf@...g.ch>,
Alexander Duyck <alexander.h.duyck@...hat.com>,
Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
Herbert Xu <herbert@...dor.apana.org.au>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords
On Thu, Jul 16, 2015 at 5:40 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
> 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
>
jhash is used in several places in the critical data path. Does the
decrease in text size justify performance impact of not inlining it?
Tom
> 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 <dvlasenk@...hat.com>
> CC: Thomas Graf <tgraf@...g.ch>
> CC: Alexander Duyck <alexander.h.duyck@...hat.com>
> CC: Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>
> CC: Herbert Xu <herbert@...dor.apana.org.au>
> CC: netdev@...r.kernel.org
> CC: linux-kernel@...r.kernel.org
> ---
> Changes in v2: created a new source file, jhash.c
>
> include/linux/jhash.h | 123 +----------------------------------------
> lib/Makefile | 2 +-
> lib/jhash.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/rhashtable.c | 13 +++--
> 4 files changed, 160 insertions(+), 127 deletions(-)
> create mode 100644 lib/jhash.c
>
> 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/Makefile b/lib/Makefile
> index 6897b52..978be53 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -26,7 +26,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
> bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \
> gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \
> bsearch.o find_bit.o llist.o memweight.o kfifo.o \
> - percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
> + percpu-refcount.o percpu_ida.o jhash.o rhashtable.o reciprocal_div.o
> obj-y += string_helpers.o
> obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
> obj-y += hexdump.o
> diff --git a/lib/jhash.c b/lib/jhash.c
> new file mode 100644
> index 0000000..cf3c277
> --- /dev/null
> +++ b/lib/jhash.c
> @@ -0,0 +1,149 @@
> +/* Jenkins hash support.
> + *
> + * Copyright (C) 2006. Bob Jenkins (bob_jenkins@...tleburtle.net)
> + *
> + * http://burtleburtle.net/bob/hash/
> + *
> + * These are the credits from Bob's sources:
> + *
> + * lookup3.c, by Bob Jenkins, May 2006, Public Domain.
> + *
> + * These are functions for producing 32-bit hashes for hash table lookup.
> + * hashword(), hashlittle(), hashlittle2(), hashbig(), mix(), and final()
> + * are externally useful functions. Routines to test the hash are included
> + * if SELF_TEST is defined. You can use this free for any purpose. It's in
> + * the public domain. It has no warranty.
> + *
> + * Copyright (C) 2009-2010 Jozsef Kadlecsik (kadlec@...ckhole.kfki.hu)
> + *
> + * I've modified Bob's hash to be useful in the Linux kernel, and
> + * any bugs present are my fault.
> + * Jozsef
> + */
> +#include <linux/export.h>
> +#include <linux/jhash.h>
> +
> +/* __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);
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index cc0c697..cf429c5 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -663,11 +663,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 +768,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists