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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxx>
> Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
> Cc: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> Cc: linux-nfs@xxxxxxxxxxxxxxx
> ---
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
