Re: [PATCH 00/10] String hash improvements

Re: [PATCH 00/10] String hash improvements

[George Spelvin]

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.

Re: [PATCH 00/10] String hash improvements

[Geert Uytterhoeven]

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

Re: [PATCH 00/10] String hash improvements

[George Spelvin]

>> +#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!

[PATCH 00/10] String hash improvements

[George Spelvin]

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

Re: [PATCH 00/10] String hash improvements

[Geert Uytterhoeven]

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

Re: [PATCH 00/10] String hash improvements

[Linus Torvalds]

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

Re: [PATCH 00/10] String hash improvements

[Fubo Chen]

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.