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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[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 CC: Thomas Graf CC: Alexander Duyck CC: Jozsef Kadlecsik CC: Herbert Xu CC: linux-kernel@vger.kernel.org --- 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