From: "George Spelvin" <linux@sciencehorizons.net>
To: linux@sciencehorizons.net, peterz@infradead.org
Cc: bfields@redhat.com, linux-kernel@vger.kernel.org,
torvalds@linux-foundation.org
Subject: Re: [PATCH v3 06/10] fs/namei.c: Improve dcache hash function
Date: 30 May 2016 12:06:18 -0400 [thread overview]
Message-ID: <20160530160618.22150.qmail@ns.sciencehorizons.net> (raw)
In-Reply-To: <20160530151129.GO3206@twins.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> wrote:
> On Sat, May 28, 2016 at 03:57:19PM -0400, George Spelvin wrote:
>> +static inline unsigned int fold_hash(unsigned long x, unsigned long y)
>> {
>> + y ^= x * GOLDEN_RATIO_64;
>> + y *= GOLDEN_RATIO_64;
>> + return y >> 32;
>> }
> So does it make sense to use that pattern here too?
>
> This code doesn't much care about performance, but wants a decent hash
> from the stack of class keys.
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 81f1a7107c0e..c8498efcd5d9 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -309,10 +309,12 @@ static struct hlist_head chainhash_table[CHAINHASH_SIZE];
> * It's a 64-bit hash, because it's important for the keys to be
> * unique.
> */
> -#define iterate_chain_key(key1, key2) \
> - (((key1) << MAX_LOCKDEP_KEYS_BITS) ^ \
> - ((key1) >> (64-MAX_LOCKDEP_KEYS_BITS)) ^ \
> - (key2))
> +static inline u64 iterate_chain_key(u64 x, u64 y)
> +{
> + y ^= x * GOLDEN_RATIO_64;
> + y *= GOLDEN_RATIO_64;
> + return y;
> +}
>
> void lockdep_off(void)
> {
Not quite. The fold_hash() you quote is used only on 64-bit systems,
which can be assumed to have a reasonable 64-bit multiply. On 32-bit
platforms, I avoid using GOLDEN_RATIO_64 at all, since 64x64-bit
multiplies are so expensive.
You actually have only 96 bits of input. The correct prototype is:
static inline u64 iterate_chain_key(u64 key, u32 idx)
If performance mattered, I'd be inclined to use one or two iterations
of the 32-bit HASH_MIX() function, which is specifically designed
to add 32 bits to a 64-bit hash value.
A more thorough mixing would be achieved by __jhash_mix(). Basically:
static inline u64 iterate_chain_key(u64 key, u32 idx)
{
u32 k0 = key, k1 = key >> 32;
__jhash_mix(idx, k0, k1) /* Macro that modifies arguments! */
return k0 | (u64)k1 << 32;
}
(The order of arguments is chosen to perserve the two "most-hashed" values.)
Also, I just had contact from the hppa folks who have brought to my
attention that it's an example of an out-of-order superscalar CPU that
*doesn't* have a good integer multiplier. For general multiplies,
you have to move values to the FPU and the code is a pain.
Instead, it has shift-and-add instructions designed to help the compiler
generate multiplies by constants, but large ones like GOLDEN_RATIO_64
is still a pain.
Here's code to take x (in %r26) and multiply it by GOLDEN_RATIO_64,
with the result in %r28:
shladd,l %r26,1,%r26,%r19
depd,z %r19,46,47,%r31
sub %r31,%r19,%r31
shladd,l %r31,2,%r26,%r31
shladd,l %r31,2,%r26,%r31
shladd,l %r31,2,%r31,%r19
depd,z %r19,60,61,%r31
sub %r31,%r19,%r31
depd,z %r31,54,55,%r28
add,l %r31,%r28,%r31
shladd,l %r31,3,%r26,%r31
depd,z %r31,59,60,%r28
add,l %r31,%r28,%r31
shladd,l %r31,2,%r26,%r31
depd,z %r31,60,61,%r28
sub %r28,%r31,%r28
depd,z %r28,60,61,%r31
sub %r31,%r28,%r31
depd,z %r31,57,58,%r28
add,l %r31,%r28,%r28
depd,z %r28,61,62,%r28
sub %r28,%r26,%r28
shladd,l %r28,3,%r28,%r28
We're going to work on that. Either finding a better code sequence,
or using the 32-bit version of hash_64() witht he nice code I've already
found for multiplies by GOLDEN_RATIO_32.
(But thaks, Peter, for looking a the code!)
next prev parent reply other threads:[~2016-05-30 16:06 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CA+55aFxPSW+84KfQ1N_WmND-wtvgj2zQm8nFPkRcc+gyU=uing@mail.gmail.com>
2016-05-25 7:20 ` [PATCH 00/10] String hash improvements George Spelvin
2016-05-25 8:00 ` Geert Uytterhoeven
2016-05-25 8:11 ` George Spelvin
2016-05-25 8:50 ` Geert Uytterhoeven
2016-05-25 9:07 ` George Spelvin
2016-05-25 16:08 ` Linus Torvalds
2016-05-28 19:57 ` [PATCH v3 " George Spelvin
2016-05-28 19:57 ` [PATCH v3 01/10] Pull out string hash to <linux/stringhash.h> George Spelvin
2016-05-28 19:57 ` [PATCH v3 02/10] fs/namei.c: Add hashlen_string() function George Spelvin
2016-05-28 19:57 ` [PATCH v3 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hashlen_string() George Spelvin
2016-05-28 19:57 ` [PATCH v3 04/10] Change hash_64() return value to 32 bits George Spelvin
2016-05-28 19:57 ` [PATCH v3 05/10] Eliminate bad hash multipliers from hash_32() and hash_64() George Spelvin
2016-05-28 19:57 ` [PATCH v3 06/10] fs/namei.c: Improve dcache hash function George Spelvin
2016-05-30 15:11 ` Peter Zijlstra
2016-05-30 16:06 ` George Spelvin [this message]
2016-05-30 16:27 ` Peter Zijlstra
2016-05-30 18:10 ` George Spelvin
2016-06-02 1:18 ` Linus Torvalds
2016-06-02 2:31 ` George Spelvin
2016-06-02 16:35 ` Linus Torvalds
2016-06-02 18:23 ` George Spelvin
2016-05-28 19:57 ` [PATCH v3 07/10] <linux/hash.h>: Add support for architecture-specific functions George Spelvin
2016-05-29 7:57 ` Geert Uytterhoeven
2016-05-28 19:57 ` [PATCH v3 08/10] m68k: Add <asm/hash.h> George Spelvin
2016-05-28 19:57 ` [PATCH v3 09/10] microblaze: " George Spelvin
2016-05-28 19:57 ` [PATCH v3 10/10] h8300: " George Spelvin
2016-05-28 20:47 ` [PATCH v3 00/10] String hash improvements Linus Torvalds
2016-05-28 20:54 ` George Spelvin
2016-06-02 22:59 ` [PATCH " Fubo Chen
2016-05-26 17:09 ` [PATCH v2 " George Spelvin
2016-05-25 7:21 ` [PATCH 01/10] Pull out string hash to <linux/stringhash.h> George Spelvin
2016-05-25 7:22 ` [PATCH 02/10] fs/namei.c: Add hash_string() function George Spelvin
2016-05-25 7:26 ` [PATCH 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string() George Spelvin
2016-05-25 7:28 ` [PATCH 04/10] Change hash_64() return value to 32 bits George Spelvin
2016-05-25 7:29 ` [PATCH 05/10] Eliminate bad hash multipliers from hash_32() and hash_64() George Spelvin
2016-05-25 7:31 ` [PATCH 06/10] fs/namei.c: Improve dcache hash function George Spelvin
2016-05-25 7:33 ` [PATCH 07/10] <linux/hash.h>: Add support for architecture-specific functions George Spelvin
2016-05-26 17:16 ` [PATCH v2 " George Spelvin
2016-05-25 7:34 ` [PATCH 08/10] m68k: Add <asm/archhash.h> George Spelvin
2016-05-25 7:34 ` George Spelvin
2016-05-25 8:07 ` Geert Uytterhoeven
2016-05-25 8:19 ` George Spelvin
2016-05-25 8:24 ` [PATCH 08v2/10] " George Spelvin
2016-05-25 8:48 ` Geert Uytterhoeven
2016-05-25 8:56 ` [PATCH 08/10] " Philippe De Muyter
2016-05-25 9:14 ` George Spelvin
2016-05-25 9:31 ` Andreas Schwab
2016-05-25 9:51 ` Philippe De Muyter
2016-05-25 13:24 ` Philippe De Muyter
2016-05-25 13:42 ` George Spelvin
2016-05-26 17:19 ` [PATCH v2 08/10] m68k: Add <asm/hash.h> George Spelvin
2016-05-25 7:37 ` [PATCH 09/10] microblaze: Add <asm/archhash.h> George Spelvin
2016-05-26 17:21 ` [PATCH v2 09/10] microblaze: Add <asm/hash.h> George Spelvin
2016-05-25 7:38 ` [PATCH 10/10] h8300: Add <asm/archhash.h> George Spelvin
2016-05-26 17:23 ` [PATCH v2 10/10] h8300: Add <asm/hash.h> George Spelvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160530160618.22150.qmail@ns.sciencehorizons.net \
--to=linux@sciencehorizons.net \
--cc=bfields@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).