From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg KH Subject: Re: Name hashing function causing a perf regression Date: Mon, 15 Sep 2014 09:35:54 -0700 Message-ID: <20140915163554.GC15712@kroah.com> References: <54134EFA.2030101@fb.com> <541364DE.5030104@fb.com> <54136EEF.4050209@fb.com> <54170C08.5070603@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Josef Bacik , stable , Andi Kleen , linux-fsdevel , Al Viro , Christoph Hellwig , Chris Mason To: Linus Torvalds Return-path: Received: from out1-smtp.messagingengine.com ([66.111.4.25]:37172 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752812AbaIOQf4 (ORCPT ); Mon, 15 Sep 2014 12:35:56 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by gateway2.nyi.internal (Postfix) with ESMTP id C956820D27 for ; Mon, 15 Sep 2014 12:35:55 -0400 (EDT) Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Sep 15, 2014 at 09:22:57AM -0700, Linus Torvalds wrote: > On Mon, Sep 15, 2014 at 8:55 AM, Josef Bacik 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 --- 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 #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) {