Re: [PATCH 0/3 v3] dcache: make it more scalable on large system

[Dave Chinner <david@fromorbit.com>]

On Wed, May 22, 2013 at 09:37:25PM -0400, Waiman Long wrote:
> Change log:
> 
> v2->v3
>  - Fix the RCU lock problem found by Al Viro.
>  - Rebase the code to the latest v3.10-rc1 linux mainline.
>  - Remove patch 4 which may be problematic if the dentry is deleted.
>  - Rerun performance measurement using 3.10-rc1 kernel.
> 
> v1->v2
>  - Include performance improvement in the AIM7 benchmark results because
>    of this patch.
>  - Modify dget_parent() to avoid taking the lock, if possible, to further
>    improve AIM7 benchmark results.
> 
> During some perf-record sessions of the kernel running the high_systime
> workload of the AIM7 benchmark, it was found that quite a large portion
> of the spinlock contention was due to the perf_event_mmap_event()
> function itself. This perf kernel function calls d_path() which,
> in turn, call path_get() and dput() indirectly. These 3 functions
> were the hottest functions shown in the perf-report output of
> the _raw_spin_lock() function in an 8-socket system with 80 cores
> (hyperthreading off) with a 3.10-rc1 kernel:

<sigh>

What was it I said about this patchset when you posted it to speed
up an Oracle benchmark back in february? I'll repeat:

"Nobody should be doing reverse dentry-to-name lookups in a quantity
sufficient for it to become a performance limiting factor."

And that makes whatever that tracepoint is doing utterly stupid.
Dumping out full paths in a tracepoint is hardly "low overhead", and
that tracepoint should be stomped on from a great height. Sure,
print the filename straight out of the dentry into a tracepoint,
but repeated calculating the full path (probably for the same set of
dentries) is just a dumb thing to do.

Anyway, your numbers show that a single AIM7 microbenchmark goes
better under tracing the specific mmap event that uses d_path(), but
the others are on average a little bit slower. That doesn't convince
me that this is worth doing. Indeed, what happens to performance
when you aren't tracing?

Indeed, have you analysed what makes that
microbenchmark contend so much on the dentry lock while reverse
lookups are occuring? Dentry lock contention does not necessarily
indicate a problem with the dentry locks, and without knowing why
there is contention occuring (esp. compared to the other benchmarks)
we can't really determine if this is a good solution or not...

IOWs, you need more than one microbenchmark that interacts with
some naive monitoring code to justify the complexity these locking
changes introduce....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com