Re: Name hashing function causing a perf regression

[Josef Bacik <jbacik@fb.com>]

On 09/14/2014 09:32 PM, Linus Torvalds wrote:
> On Sat, Sep 13, 2014 at 11:58 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So if this all helps you guys, and after sufficient testing, just the
>> current top commit should be self-sufficient (with two other commits
>> before *if* that multiply-vs-shift makes a difference)
>>
>>      99d263d4c5b2 (HEAD, master) vfs: fix bad hashing of dentries
>>
>> and if it *doesn't* help, and you still see problems, please holler.
>
> Btw, please if at all possible, give the 3.17-rc5 release a test on
> the load that you saw problems with.
>
> Because of the whole dentry hashing issue, I took another look at name
> lookup performance, and found a few more problems in this area.
>
> The biggest problem only affected the fairly unusual case of a
> directory that was mounted differently in different namespaces, and it
> may well be that you don't actually have that situation at all. I
> found it almost by mistake when checking performance consistency and
> noticing that my "/tmp" directory lookups were much slower than
> everything else. The pathname lookup incorrectly dropped out of RCU
> mode for that case due to two independent bugs (one hit normal lookups
> of such directories, the other hit just the ".." case).
>
> I also hit a small CPU pipeline hickup in link_path_walk() that is
> probably specific to just the store buffer forwarding of x86-64, but
> could possibly hit other 64-bit cases too. I doubt it's noticeable for
> your case, but it showed up pretty clearly in the profiles when I was
> checking that everything looked ok.
>
> All of them should be fixed in the -rc5 I just pushed out. At least I
>
> I do have another case I'm not entirely happy about - our negative
> lookups (ie looking up a pathname that doesn't exist) hit in the
> dcache for real filesystems and perform really well, but they suck for
> tmpfs. Al, we turn off negative dentry caches for tmpfs because
> simple_dentry_operations uses
>
>          .d_delete = always_delete_dentry,
>
> Do we care? It's noticeable in benchmarks: it's almost an order of
> magnitude difference when looking up non-existent files. I can look up
> a non-existent file 23M times per second on ext4, but only 3.3M on
> /tmp.
>
> Anyway, I'm not sure FB does a lot of lookups of nonexistent files,
> but there are some loads that really do that a lot.  And we're in the
> odd situation that it's actually *much* faster on a real filesystem
> than it is on a RAM filesystem like /tmp.
>

Sorry, after my last reply I promptly went out of town for the weekend. 
  Ran everything again, here are the numbers (avg ops/sec)

		3.2 hashing	3.10 hashing	3.10+Your fix
XFS-mkdir	18425.083984	13783.275391	18638.523438
Ext4-mkdir	16834.097656	13801.648438	16805.552734
Btrfs-mkdir	48617.929688	25772.544922	47906.093750

XFS-rename	17321.025391	10300.987305	16647.900391
Ext4-rename	17619.650391	11027.894531	17456.312500
Btrfs-rename	26019.062500	9964.170898	25311.501953

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.

Josef