diff mbox

Name hashing function causing a perf regression

Message ID 20140915163554.GC15712@kroah.com
State New
Headers show

Commit Message

Greg KH Sept. 15, 2014, 4:35 p.m. UTC 
On Mon, Sep 15, 2014 at 09:22:57AM -0700, Linus Torvalds wrote:
> On Mon, Sep 15, 2014 at 8:55 AM, Josef Bacik <jbacik@fb.com> wrote:
> >
> > I can't test on 3.17 proper since the Fusion IO driver doesn't build
> > properly there and I'm not being paid to work on it anymore so I'm not
> > fixing it ;).  Thanks for fixing this, I've pulled back 99d263d4c5b2 which
> > will do us just fine.
> 
> Ok. I'm cc'ing stable to know that
> 
>     99d263d4c5b2 ("vfs: fix bad hashing of dentries")
> 
> should be added to the queues for 3.10+.
> 
> Greg&co - it's a simple fix for a performance regression. Not
> end-of-the-world, but if it ends up being in the FB kernel trees,
> might as well get it back-ported to the other stable trees too.
> 
> The other performance issues I found are actually potentially worse,
> but they aren't regressions and they hit only when using namespaces.
> Which mostly nobody does on old kernels anyway. More of a "systemd
> uses namespaces for /tmp, and then it's quite noticeable as slowing
> down pathname lookups there if you benchmark it".
> 
> So the single commit Josef mentions is likely sufficient for stable.

Thanks, now queued up.  Note, I had to change the 3.10-stable patch a
bit due to some rejects.

Here's a diff of the diffs to show the difference if anyone wants to
check it.

thanks,

greg k-h


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Linus Torvalds Sept. 15, 2014, 4:45 p.m. UTC | #1 
On Mon, Sep 15, 2014 at 9:35 AM, Greg KH <greg@kroah.com> wrote:
>
> Thanks, now queued up.  Note, I had to change the 3.10-stable patch a
> bit due to some rejects.

Ahh. Yes. it still uses the old D_HASHBITS/HASHMASK macros in up until
and including 3.12.

You could have just backported 482db9066199 ("dcache.c: get rid of
pointless macros") too, but your resolution looks perfectly fine too.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Slaby Sept. 15, 2014, 4:53 p.m. UTC | #2 
On 09/15/2014, 06:45 PM, Linus Torvalds wrote:
> On Mon, Sep 15, 2014 at 9:35 AM, Greg KH <greg@kroah.com> wrote:
>>
>> Thanks, now queued up.  Note, I had to change the 3.10-stable patch a
>> bit due to some rejects.
> 
> Ahh. Yes. it still uses the old D_HASHBITS/HASHMASK macros in up until
> and including 3.12.
> 
> You could have just backported 482db9066199 ("dcache.c: get rid of
> pointless macros") too

Ok, I went this path for 3.12 as 482db9066199 is simple enough. Thanks!
Greg KH Sept. 15, 2014, 5:31 p.m. UTC | #3 
On Mon, Sep 15, 2014 at 09:45:40AM -0700, Linus Torvalds wrote:
> On Mon, Sep 15, 2014 at 9:35 AM, Greg KH <greg@kroah.com> wrote:
> >
> > Thanks, now queued up.  Note, I had to change the 3.10-stable patch a
> > bit due to some rejects.
> 
> Ahh. Yes. it still uses the old D_HASHBITS/HASHMASK macros in up until
> and including 3.12.
> 
> You could have just backported 482db9066199 ("dcache.c: get rid of
> pointless macros") too, but your resolution looks perfectly fine too.

Ah, much easier, I've gone and done that now, thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- queue-3.16/vfs-fix-bad-hashing-of-dentries.patch	2014-09-15 09:27:57.072950053 -0700
+++ queue-3.10/vfs-fix-bad-hashing-of-dentries.patch	2014-09-15 09:29:06.851839582 -0700
@@ -76,13 +76,13 @@ 
 
 --- a/fs/dcache.c
 +++ b/fs/dcache.c
-@@ -106,8 +106,7 @@ static inline struct hlist_bl_head *d_ha
+@@ -108,8 +108,7 @@ static inline struct hlist_bl_head *d_ha
  					unsigned int hash)
  {
  	hash += (unsigned long) parent / L1_CACHE_BYTES;
--	hash = hash + (hash >> d_hash_shift);
--	return dentry_hashtable + (hash & d_hash_mask);
-+	return dentry_hashtable + hash_32(hash, d_hash_shift);
+-	hash = hash + (hash >> D_HASHBITS);
+-	return dentry_hashtable + (hash & D_HASHMASK);
++	return dentry_hashtable + hash_32(hash, D_HASHBITS);
  }
  
  /* Statistics gathering. */
@@ -96,7 +96,7 @@ 
  #include <asm/uaccess.h>
  
  #include "internal.h"
-@@ -1629,8 +1630,7 @@ static inline int nested_symlink(struct
+@@ -1647,8 +1648,7 @@ static inline int can_lookup(struct inod
  
  static inline unsigned int fold_hash(unsigned long hash)
  {