linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] String hash improvements
       [not found] <CA+55aFxPSW+84KfQ1N_WmND-wtvgj2zQm8nFPkRcc+gyU=uing@mail.gmail.com>
@ 2016-05-25  7:20 ` George Spelvin
  2016-05-25  8:00   ` Geert Uytterhoeven
  2016-05-25 16:08   ` Linus Torvalds
  2016-05-25  7:26 ` [PATCH 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string() George Spelvin
  1 sibling, 2 replies; 11+ messages in thread
From: George Spelvin @ 2016-05-25  7:20 UTC (permalink / raw)
  To: linux-kernel, torvalds
  Cc: alistair.francis, bfields, geert, gerg, jlayton, linux-m68k,
	linux-nfs, linux, michal.simek, tglx, uclinux-h8-devel, ysato

On Tue, 17 May 2016 at 09:32, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Tue, May 17, 2016 at 6:41 AM, George Spelvin <linux@horizon.com> wrote:
>> I had assumed that since they weren't fully baked when the window opened,
>> they weren't eligible, but I'll try.

> Hey, if they aren't ready, they aren't.

Well, they're close, and I can and did *get* them ready.

> How about just the minimal set of patches that you'er happy with as-is?

The things are a bit interdependent.  I can't fix hash_64() on 32-bit systems until
I get rid of hash_string()'s need for it to return 64 bits, which requires work
on the dcache hashes to make them suitable replacements...

The real fun has come from TPTB deciding to sell the horizon.com domain,
and it turns out that updating rDNS takes the ISP a whole freaking week,
during which time outgoing mail trips everyone's spam filters.

That finally got fixed, just in time for me to put my dominant hand through
a piece of glass.  It's been a week. :-(


Anyway, the patches...

This series does several related things:
1) Gets rid of the string hashes in <linux/sunrpc/svcauth.h>,
   and uses the dcache hash (fs/namei.c) instead.
2) Avoid 64-bit multiplies in hash_64() on 32-bit platforms.
   Two 32-bit multiplies will do well enough.
3) Rids the world of the bad hash multipliers in hash_32.
   This finishes the job started in 689de1d6ca.
   The vast majority of Linux architectures have hardware support
   for 32x32-bit multiply and so derive no benefit from "simplified"
   multipliers.
   The few processors that do not (68000, h8/300 and some models of
   Microblaze) have arch-specific implementations added.  Those patches
   are last in the series so they can go through the relevant arch
   maintainers.
4) Overhauls the dcache hash mixing.
   The patch in 2bf0b16954 was an off-the-cuff suggestion.  Replaced with
   a much more careful design that's simultaneously faster and better.
   (My own invention, as there was noting suitable in the literature I
   could find.  Comments welcome!)

Things I thought about but can wait for now:
5) Modify the hash_name() loop to skip the initial HASH_MIX().
   That would let us salt the hash if we ever wanted to.
6) Modify bytemask_from_count to handle inputs of 1..sizeof(long)
   rather than 0..sizeof(long)-1.  This would simplify all its users
   including full_name_hash.
7) Sort out partial_name_hash().
   The hash function is declared as using a long state, even though
   it's truncated to 32 bits at the end and the extra internal state
   contributes nothing to the result.  And some callers do odd things:
   * fs/hfs/string.c only allocates 32 bits of state
   * fs/hfsplus/unicode.c uses it to hash 16-bit unicode symbols not bytes

I'm not particularly fond of the names of the header files I created,
but if anyone has a better idea please talk fast!

George Spelvin (10):
  Pull out string hash to <linux/stringhash.h>
  fs/namei.c: Add hash_string() function.
  <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string()
  Change hash_64() return value to 32 bits.
  Eliminate bad hash multipliers from hash_32() and hash_64().
  fs/namei.c: Improve dcache hash function
  <linux/hash.h>: Add support for architecture-specific functions
  m68k: Add <asm/archhash.h>
  microblaze: Add <asm/archhash.h>
  h8300: Add <asm/archhash.h>

 arch/Kconfig                           |   8 ++
 arch/h8300/Kconfig                     |   1 +
 arch/h8300/include/asm/archhash.h      |  52 ++++++++++++
 arch/m68k/Kconfig                      |   1 +
 arch/m68k/include/asm/archhash.h       |  67 +++++++++++++++
 arch/microblaze/Kconfig                |   1 +
 arch/microblaze/include/asm/archhash.h |  80 ++++++++++++++++++
 fs/namei.c                             | 149 +++++++++++++++++++++++++--------
 include/linux/dcache.h                 |  27 +-----
 include/linux/hash.h                   | 111 ++++++++++++------------
 include/linux/stringhash.h             |  76 +++++++++++++++++
 include/linux/sunrpc/svcauth.h         |  36 ++------
 12 files changed, 464 insertions(+), 145 deletions(-)
 create mode 100644 arch/h8300/include/asm/archhash.h
 create mode 100644 arch/m68k/include/asm/archhash.h
 create mode 100644 arch/microblaze/include/asm/archhash.h
 create mode 100644 include/linux/stringhash.h

-- 
2.8.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string()
       [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  7:26 ` George Spelvin
  2016-05-26 18:39   ` [PATCH RESEND " George Spelvin
  1 sibling, 1 reply; 11+ messages in thread
From: George Spelvin @ 2016-05-25  7:26 UTC (permalink / raw)
  To: bfields, jlayton, linux-kernel, linux-nfs, torvalds; +Cc: linux, tglx

Finally, the first use of previous two patches: Eliminate the
separate ad-hoc string hash functions in the sunrpc code.

This also eliminates the only caller of hash_long which asks for
more than 32 bits of output.

sunrpc guys: Is it okay if I send this to Linus directly?

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: linux-nfs@vger.kernel.org
---
 include/linux/sunrpc/svcauth.h | 36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index c00f53a4..ef2b2552 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -16,6 +16,7 @@
 #include <linux/sunrpc/cache.h>
 #include <linux/sunrpc/gss_api.h>
 #include <linux/hash.h>
+#include <linux/stringhash.h>
 #include <linux/cred.h>
 
 struct svc_cred {
@@ -165,41 +166,14 @@ extern int svcauth_unix_set_client(struct svc_rqst *rqstp);
 extern int unix_gid_cache_create(struct net *net);
 extern void unix_gid_cache_destroy(struct net *net);
 
-static inline unsigned long hash_str(char *name, int bits)
+static inline unsigned long hash_str(char const *name, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (unlikely(!(c = *name++))) {
-			c = (char)len; len = -1;
-		}
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return hash_32(hashlen_hash(hash_string(name)), bits);
 }
 
-static inline unsigned long hash_mem(char *buf, int length, int bits)
+static inline unsigned long hash_mem(char const *buf, int length, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (len == length) {
-			c = (char)len; len = -1;
-		} else
-			c = *buf++;
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return hash_32(full_name_hash(buf, length), bits);
 }
 
 #endif /* __KERNEL__ */
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  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 16:08   ` Linus Torvalds
  1 sibling, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2016-05-25  8:00 UTC (permalink / raw)
  To: George Spelvin
  Cc: linux-kernel@vger.kernel.org, Linus Torvalds, alistair.francis,
	Bruce Fields, Greg Ungerer, Jeff Layton, linux-m68k,
	open list:NFS, SUNRPC, AND..., Michal Simek, Thomas Gleixner,
	uclinux-h8-devel, Yoshinori Sato

Hi George,

On Wed, May 25, 2016 at 9:20 AM, George Spelvin
<linux@sciencehorizons.net> wrote:
> I'm not particularly fond of the names of the header files I created,
> but if anyone has a better idea please talk fast!

Usually this is handled through include/asm-generic/.
Put the generic default implementation in include/asm-generic/hash.h.

Architectures that need to override provide their own version, e.g.
arch/m68k/include/asm/hash.h. They may #include <asm-generic/hash.h>
if they still want to reuse parts of the generic implementation.

Other architectures add "generic-y += hash.h" to their
arch/<ARCH>/include/asm/Kbuild.

<linux/hash.h> includes <asm/hash.h> t.

>  arch/h8300/include/asm/archhash.h      |  52 ++++++++++++
>  arch/m68k/include/asm/archhash.h       |  67 +++++++++++++++
>  arch/microblaze/include/asm/archhash.h |  80 ++++++++++++++++++
>  include/linux/hash.h                   | 111 ++++++++++++------------
>  include/linux/stringhash.h             |  76 +++++++++++++++++

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  2016-05-25  8:00   ` Geert Uytterhoeven
@ 2016-05-25  8:11     ` George Spelvin
  2016-05-25  8:50       ` Geert Uytterhoeven
  0 siblings, 1 reply; 11+ messages in thread
From: George Spelvin @ 2016-05-25  8:11 UTC (permalink / raw)
  To: geert, linux
  Cc: alistair.francis, bfields, gerg, jlayton, linux-kernel,
	linux-m68k, linux-nfs, michal.simek, tglx, torvalds,
	uclinux-h8-devel, ysato

Geert Uytterhoeven wrote:
> Usually this is handled through include/asm-generic/.
> Put the generic default implementation in include/asm-generic/hash.h.
>
> Architectures that need to override provide their own version, e.g.
> arch/m68k/include/asm/hash.h. They may #include <asm-generic/hash.h>
> if they still want to reuse parts of the generic implementation.
>
> Other architectures add "generic-y += hash.h" to their
> arch/<ARCH>/include/asm/Kbuild.

I thought about that, but then I'd have to edit *every* architecture,
and might need acks from all the maintainers.

I was looking for something that was a total no-op on most architectures.

But if this is preferred, it's not technically difficult at all.


If asm-generic were in the <asm/*.h> search path, it would magically
Just Work, but leftover files from a broken checkout would be a big
potential problem.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  2016-05-25  8:11     ` George Spelvin
@ 2016-05-25  8:50       ` Geert Uytterhoeven
  2016-05-25  9:07         ` George Spelvin
  0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2016-05-25  8:50 UTC (permalink / raw)
  To: George Spelvin
  Cc: alistair.francis, Bruce Fields, Greg Ungerer, Jeff Layton,
	linux-kernel@vger.kernel.org, linux-m68k,
	open list:NFS, SUNRPC, AND..., Michal Simek, Thomas Gleixner,
	Linus Torvalds, uclinux-h8-devel, Yoshinori Sato

On Wed, May 25, 2016 at 10:11 AM, George Spelvin
<linux@sciencehorizons.net> wrote:
> Geert Uytterhoeven wrote:
>> Usually this is handled through include/asm-generic/.
>> Put the generic default implementation in include/asm-generic/hash.h.
>>
>> Architectures that need to override provide their own version, e.g.
>> arch/m68k/include/asm/hash.h. They may #include <asm-generic/hash.h>
>> if they still want to reuse parts of the generic implementation.
>>
>> Other architectures add "generic-y += hash.h" to their
>> arch/<ARCH>/include/asm/Kbuild.
>
> I thought about that, but then I'd have to edit *every* architecture,
> and might need acks from all the maintainers.
>
> I was looking for something that was a total no-op on most architectures.
>
> But if this is preferred, it's not technically difficult at all.

As you only include <asm/archhash.h> if CONFIG_HAVE_ARCH_HASH
is defined, you can also just call the arch-specific one <asm/hash.h>.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  2016-05-25  8:50       ` Geert Uytterhoeven
@ 2016-05-25  9:07         ` George Spelvin
  0 siblings, 0 replies; 11+ messages in thread
From: George Spelvin @ 2016-05-25  9:07 UTC (permalink / raw)
  To: geert, linux
  Cc: alistair.francis, bfields, gerg, jlayton, linux-kernel,
	linux-m68k, linux-nfs, michal.simek, tglx, torvalds,
	uclinux-h8-devel, ysato

>> +#if defined(CONFIG_M68000) || defined(CONFIG_M68010)

> As I said before, I don't think you need this check, given HAVE_ARCH_HASH is
> selected by M68000, and M68010 doesn't exist.

I was going belt & suspenders on general principles, but yes, I'm happy
to leave it out.

I noticed that CONFIG_M68010 doesn't exist in Linus' tree, but you
recommended it, so I thought you might know something I don't.

> As you only include <asm/archhash.h> if CONFIG_HAVE_ARCH_HASH
> is defined, you can also just call the arch-specific one <asm/hash.h>.

Yes, that's a possibility, too.  But weren't we still discussing whether
I should use conditional #inclusion based on a symbol, or asm-generic?

If neither of us has a killer argument that convinces the other, style
issues like this are amenable to voting, so I was going to wait a little
bit for others to chime in.


Thank you very much for the comments!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  2016-05-25  7:20 ` [PATCH 00/10] String hash improvements George Spelvin
  2016-05-25  8:00   ` Geert Uytterhoeven
@ 2016-05-25 16:08   ` Linus Torvalds
       [not found]     ` <1464465443-25305-1-git-send-email-linux@sciencehorizons.net>
  2016-06-02 22:59     ` [PATCH 00/10] String hash improvements Fubo Chen
  1 sibling, 2 replies; 11+ messages in thread
From: Linus Torvalds @ 2016-05-25 16:08 UTC (permalink / raw)
  To: George Spelvin
  Cc: lkml, alistair.francis, Bruce Fields, geert, gerg, jlayton,
	linux-m68k, linux-nfs, michal.simek, Thomas Gleixner,
	uclinux-h8-devel, ysato

On Wed, May 25, 2016 at 12:20 AM, George Spelvin
<linux@sciencehorizons.net> wrote:
>
> Well, they're close, and I can and did *get* them ready.

Ok, thanks. For some odd reason all your emails in this series got
marked as spam. Every single one, including the cover letter (but not
your replies to the replies to this).

Stupidly, I unmarked them before thinking to ask google *why* they got
marked as spam.

Did you do something different to send this series than you usually do?

Would you mind sending it again (maybe you have changes due to some o
the comments, but even if not, I'd like to see what the spam filter
says)? You could do it just to me to avoid filling up the mailing list
with duplicates.

                    Linus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH RESEND 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string()
  2016-05-25  7:26 ` [PATCH 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string() George Spelvin
@ 2016-05-26 18:39   ` George Spelvin
  2016-05-26 18:45     ` J. Bruce Fields
  0 siblings, 1 reply; 11+ messages in thread
From: George Spelvin @ 2016-05-26 18:39 UTC (permalink / raw)
  To: bfields, jlayton, linux-nfs; +Cc: linux

Finally, the first use of previous two patches: Eliminate the
separate ad-hoc string hash functions in the sunrpc code.

This also eliminates the only caller of hash_long which asks for
more than 32 bits of output.

sunrpc guys: Is it okay if I send this to Linus directly?

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: linux-nfs@vger.kernel.org
---
I have't heard anything from the linix-NFS crowd, so this is a re-ping.

I've been having some e-mail troubles leading to increased spam scores
due to a recent domain name change, so this is a resend from a different
address in case the previous copy got binned.

I'll happily send along the entire patch series, but the tl;dr is this
is part of a patch series that improves the <linux/hash.h> and fs/namei.c
hash functions, implementing Linus's suggestion:

On Mon, 02 May 2016 at 09:24:21, Linus Torvalds wrote:
> That said, I actually think hash_str() should be killed entirely.
> Better just use what we use for pathnames: full_name_hash() (which
> gets a pointer and length) and hash_name (which gets the string).
> 
> Those functions do the "word-at-a-time" optimization, and they should
> do a good enough job. If they aren't, we should fix them, because they
> are a hell of a lot more important than anything that the svcauth code
> does.

He had a typically fulminating comment about the hard-to-predict if()
branch in the inner loop.

 include/linux/sunrpc/svcauth.h | 36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index c00f53a4..ef2b2552 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -16,6 +16,7 @@
 #include <linux/sunrpc/cache.h>
 #include <linux/sunrpc/gss_api.h>
 #include <linux/hash.h>
+#include <linux/stringhash.h>
 #include <linux/cred.h>
 
 struct svc_cred {
@@ -165,41 +166,14 @@ extern int svcauth_unix_set_client(struct svc_rqst *rqstp);
 extern int unix_gid_cache_create(struct net *net);
 extern void unix_gid_cache_destroy(struct net *net);
 
-static inline unsigned long hash_str(char *name, int bits)
+static inline unsigned long hash_str(char const *name, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (unlikely(!(c = *name++))) {
-			c = (char)len; len = -1;
-		}
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return hash_32(hashlen_hash(hash_string(name)), bits);
 }
 
-static inline unsigned long hash_mem(char *buf, int length, int bits)
+static inline unsigned long hash_mem(char const *buf, int length, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (len == length) {
-			c = (char)len; len = -1;
-		} else
-			c = *buf++;
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return hash_32(full_name_hash(buf, length), bits);
 }
 
 #endif /* __KERNEL__ */
-- 
2.8.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH RESEND 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string()
  2016-05-26 18:39   ` [PATCH RESEND " George Spelvin
@ 2016-05-26 18:45     ` J. Bruce Fields
  0 siblings, 0 replies; 11+ messages in thread
From: J. Bruce Fields @ 2016-05-26 18:45 UTC (permalink / raw)
  To: linux; +Cc: jlayton, linux-nfs

On Thu, May 26, 2016 at 02:39:39PM -0400, George Spelvin wrote:
> Finally, the first use of previous two patches: Eliminate the
> separate ad-hoc string hash functions in the sunrpc code.
> 
> This also eliminates the only caller of hash_long which asks for
> more than 32 bits of output.
> 
> sunrpc guys: Is it okay if I send this to Linus directly?

Sorry for ignoring this.... Looks straightforward to me; for what it's
worth:

	Acked-by: J. Bruce Fields <bfields@redhat.com>

If you have a git branch I could fetch, I could also run my usual tests
on it.  (Which would only help in the unlikely case this breaks nfs
completely somehow--they certainly wouldn't catch some hash performance
regressoin.)

--b.

> 
> Signed-off-by: George Spelvin <linux@sciencehorizons.net>
> Cc: "J. Bruce Fields" <bfields@fieldses.org>
> Cc: Jeff Layton <jlayton@poochiereds.net>
> Cc: linux-nfs@vger.kernel.org
> ---
> I have't heard anything from the linix-NFS crowd, so this is a re-ping.
> 
> I've been having some e-mail troubles leading to increased spam scores
> due to a recent domain name change, so this is a resend from a different
> address in case the previous copy got binned.
> 
> I'll happily send along the entire patch series, but the tl;dr is this
> is part of a patch series that improves the <linux/hash.h> and fs/namei.c
> hash functions, implementing Linus's suggestion:
> 
> On Mon, 02 May 2016 at 09:24:21, Linus Torvalds wrote:
> > That said, I actually think hash_str() should be killed entirely.
> > Better just use what we use for pathnames: full_name_hash() (which
> > gets a pointer and length) and hash_name (which gets the string).
> > 
> > Those functions do the "word-at-a-time" optimization, and they should
> > do a good enough job. If they aren't, we should fix them, because they
> > are a hell of a lot more important than anything that the svcauth code
> > does.
> 
> He had a typically fulminating comment about the hard-to-predict if()
> branch in the inner loop.
> 
>  include/linux/sunrpc/svcauth.h | 36 +++++-------------------------------
>  1 file changed, 5 insertions(+), 31 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
> index c00f53a4..ef2b2552 100644
> --- a/include/linux/sunrpc/svcauth.h
> +++ b/include/linux/sunrpc/svcauth.h
> @@ -16,6 +16,7 @@
>  #include <linux/sunrpc/cache.h>
>  #include <linux/sunrpc/gss_api.h>
>  #include <linux/hash.h>
> +#include <linux/stringhash.h>
>  #include <linux/cred.h>
>  
>  struct svc_cred {
> @@ -165,41 +166,14 @@ extern int svcauth_unix_set_client(struct svc_rqst *rqstp);
>  extern int unix_gid_cache_create(struct net *net);
>  extern void unix_gid_cache_destroy(struct net *net);
>  
> -static inline unsigned long hash_str(char *name, int bits)
> +static inline unsigned long hash_str(char const *name, int bits)
>  {
> -	unsigned long hash = 0;
> -	unsigned long l = 0;
> -	int len = 0;
> -	unsigned char c;
> -	do {
> -		if (unlikely(!(c = *name++))) {
> -			c = (char)len; len = -1;
> -		}
> -		l = (l << 8) | c;
> -		len++;
> -		if ((len & (BITS_PER_LONG/8-1))==0)
> -			hash = hash_long(hash^l, BITS_PER_LONG);
> -	} while (len);
> -	return hash >> (BITS_PER_LONG - bits);
> +	return hash_32(hashlen_hash(hash_string(name)), bits);
>  }
>  
> -static inline unsigned long hash_mem(char *buf, int length, int bits)
> +static inline unsigned long hash_mem(char const *buf, int length, int bits)
>  {
> -	unsigned long hash = 0;
> -	unsigned long l = 0;
> -	int len = 0;
> -	unsigned char c;
> -	do {
> -		if (len == length) {
> -			c = (char)len; len = -1;
> -		} else
> -			c = *buf++;
> -		l = (l << 8) | c;
> -		len++;
> -		if ((len & (BITS_PER_LONG/8-1))==0)
> -			hash = hash_long(hash^l, BITS_PER_LONG);
> -	} while (len);
> -	return hash >> (BITS_PER_LONG - bits);
> +	return hash_32(full_name_hash(buf, length), bits);
>  }
>  
>  #endif /* __KERNEL__ */
> -- 
> 2.8.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hashlen_string()
       [not found]     ` <1464465443-25305-1-git-send-email-linux@sciencehorizons.net>
@ 2016-05-28 19:57       ` George Spelvin
  0 siblings, 0 replies; 11+ messages in thread
From: George Spelvin @ 2016-05-28 19:57 UTC (permalink / raw)
  To: Linus Torvalds, lkml
  Cc: J . Bruce Fields, George Spelvin, Jeff Layton, linux-nfs

Finally, the first use of previous two patches: eliminate the
separate ad-hoc string hash functions in the sunrpc code.

Now hash_str() is a wrapper around hash_string(), and hash_mem() is
likewise a wrapper around full_name_hash().

Note that sunrpc code *does* call hash_mem() with a zero length, which
is why the previous patch needed to handle that in full_name_hash().
(Thanks, Bruce, for finding that!)

This also eliminates the only caller of hash_long which asks for
more than 32 bits of output.

The comment about the quality of hashlen_string() and full_name_hash()
is jumping the gun by a few patches; they aren't very impressive now,
but will be improved greatly later in the series.

Signed-off-by: George Spelvin <linux@sciencehorizons.net>
Tested-by: J. Bruce Fields <bfields@redhat.com>
Acked-by: J. Bruce Fields <bfields@redhat.com>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: linux-nfs@vger.kernel.org
---
 include/linux/sunrpc/svcauth.h | 40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index c00f53a4..91d5a5d6 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -16,6 +16,7 @@
 #include <linux/sunrpc/cache.h>
 #include <linux/sunrpc/gss_api.h>
 #include <linux/hash.h>
+#include <linux/stringhash.h>
 #include <linux/cred.h>
 
 struct svc_cred {
@@ -165,41 +166,18 @@ extern int svcauth_unix_set_client(struct svc_rqst *rqstp);
 extern int unix_gid_cache_create(struct net *net);
 extern void unix_gid_cache_destroy(struct net *net);
 
-static inline unsigned long hash_str(char *name, int bits)
+/*
+ * The <stringhash.h> functions are good enough that we don't need to
+ * use hash_32() on them; just extracting the high bits is enough.
+ */
+static inline unsigned long hash_str(char const *name, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (unlikely(!(c = *name++))) {
-			c = (char)len; len = -1;
-		}
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return hashlen_hash(hashlen_string(name)) >> (32 - bits);
 }
 
-static inline unsigned long hash_mem(char *buf, int length, int bits)
+static inline unsigned long hash_mem(char const *buf, int length, int bits)
 {
-	unsigned long hash = 0;
-	unsigned long l = 0;
-	int len = 0;
-	unsigned char c;
-	do {
-		if (len == length) {
-			c = (char)len; len = -1;
-		} else
-			c = *buf++;
-		l = (l << 8) | c;
-		len++;
-		if ((len & (BITS_PER_LONG/8-1))==0)
-			hash = hash_long(hash^l, BITS_PER_LONG);
-	} while (len);
-	return hash >> (BITS_PER_LONG - bits);
+	return full_name_hash(buf, length) >> (32 - bits);
 }
 
 #endif /* __KERNEL__ */
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 00/10] String hash improvements
  2016-05-25 16:08   ` Linus Torvalds
       [not found]     ` <1464465443-25305-1-git-send-email-linux@sciencehorizons.net>
@ 2016-06-02 22:59     ` Fubo Chen
  1 sibling, 0 replies; 11+ messages in thread
From: Fubo Chen @ 2016-06-02 22:59 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: George Spelvin, lkml, alistair.francis, Bruce Fields, geert, gerg,
	jlayton, linux-m68k, linux-nfs, michal.simek, Thomas Gleixner,
	uclinux-h8-devel, ysato

On Wed, May 25, 2016 at 9:08 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Ok, thanks. For some odd reason all your emails in this series got
> marked as spam. Every single one, including the cover letter (but not
> your replies to the replies to this).

I have added the following filter to my gmail account: "never send
e-mails with [PATCH in the subject to the spam folder". That helps a
lot.

Fubo.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-06-02 22:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [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
     [not found]     ` <1464465443-25305-1-git-send-email-linux@sciencehorizons.net>
2016-05-28 19:57       ` [PATCH v3 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hashlen_string() George Spelvin
2016-06-02 22:59     ` [PATCH 00/10] String hash improvements Fubo Chen
2016-05-25  7:26 ` [PATCH 03/10] <linux/sunrpc/svcauth.h>: Define hash_str() in terms of hash_string() George Spelvin
2016-05-26 18:39   ` [PATCH RESEND " George Spelvin
2016-05-26 18:45     ` J. Bruce Fields

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).