Re: [PATCH v2 0/4] perf lock contention: Symbolize locks using slab cache names

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Nov 11, 2024 at 11:46:37AM -0800, Ian Rogers wrote:
> On Thu, Nov 7, 2024 at 10:15 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > This is to support symbolization of dynamic locks using slab
> > allocator's metadata.  The kernel support is in the bpf-next tree now.
> >
> > It provides the new "kmem_cache" BPF iterator and "bpf_get_kmem_cache"
> > kfunc to get the information from an address.  The feature detection is
> > done using BTF type info and it won't have any effect on old kernels.
> >
> > v2 changes)
> >
> >  * don't use libbpf_get_error()  (Andrii)
> >
> > v1) https://lore.kernel.org/linux-perf-users/20241105172635.2463800-1-namhyung@kernel.org
> >
> > With this change, it can show locks in a slab object like below.  I
> > added "&" sign to distinguish them from global locks.
> 
> I know the & is intentional but I worry it could later complicate
> parsing of filters. Perhaps @ is a viable alternative. Other than
> that:
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks for the review!

I don't think it clashes with BPF sample filters which works on sample
data generated from a perf_event.  Technically this command doesn't use
perf_event and just attaches the BPF program to tracepoint directly.

Also sample filters don't use '&' symbol in the syntax as of now. :)

Thanks,
Namhyung

> 
> >     # perf lock con -abl sleep 1
> >      contended   total wait     max wait     avg wait            address   symbol
> >
> >              2      1.95 us      1.77 us       975 ns   ffff9d5e852d3498   &task_struct (mutex)
> >              1      1.18 us      1.18 us      1.18 us   ffff9d5e852d3538   &task_struct (mutex)
> >              4      1.12 us       354 ns       279 ns   ffff9d5e841ca800   &kmalloc-cg-512 (mutex)
> >              2       859 ns       617 ns       429 ns   ffffffffa41c3620   delayed_uprobe_lock (mutex)
> >              3       691 ns       388 ns       230 ns   ffffffffa41c0940   pack_mutex (mutex)
> >              3       421 ns       164 ns       140 ns   ffffffffa3a8b3a0   text_mutex (mutex)
> >              1       409 ns       409 ns       409 ns   ffffffffa41b4cf8   tracepoint_srcu_srcu_usage (mutex)
> >              2       362 ns       239 ns       181 ns   ffffffffa41cf840   pcpu_alloc_mutex (mutex)
> >              1       220 ns       220 ns       220 ns   ffff9d5e82b534d8   &signal_cache (mutex)
> >              1       215 ns       215 ns       215 ns   ffffffffa41b4c28   tracepoint_srcu_srcu_usage (mutex)
> >
> > The first two were from "task_struct" slab cache.  It happened to
> > match with the type name of object but there's no guarantee.  We need
> > to add type info to slab cache to resolve the lock inside the object.
> > Anyway, the third one has no dedicated slab cache and was allocated by
> > kmalloc.
> >
> > Those slab objects can be used to filter specific locks using -L or
> >  --lock-filter option.  (It needs quotes to avoid special handling in
> > the shell).
> >
> >     # perf lock con -ab -L '&task_struct' sleep 1
> >        contended   total wait     max wait     avg wait         type   caller
> >
> >                1     25.10 us     25.10 us     25.10 us        mutex   perf_event_exit_task+0x39
> >                1     21.60 us     21.60 us     21.60 us        mutex   futex_exit_release+0x21
> >                1      5.56 us      5.56 us      5.56 us        mutex   futex_exec_release+0x21
> >
> > The code is available at 'perf/lock-slab-v2' branch in my tree
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> >
> > Namhyung Kim (4):
> >   perf lock contention: Add and use LCB_F_TYPE_MASK
> >   perf lock contention: Run BPF slab cache iterator
> >   perf lock contention: Resolve slab object name using BPF
> >   perf lock contention: Handle slab objects in -L/--lock-filter option
> >
> >  tools/perf/builtin-lock.c                     |  39 ++++-
> >  tools/perf/util/bpf_lock_contention.c         | 140 +++++++++++++++++-
> >  .../perf/util/bpf_skel/lock_contention.bpf.c  |  70 ++++++++-
> >  tools/perf/util/bpf_skel/lock_data.h          |  15 +-
> >  tools/perf/util/bpf_skel/vmlinux/vmlinux.h    |   8 +
> >  tools/perf/util/lock-contention.h             |   2 +
> >  6 files changed, 267 insertions(+), 7 deletions(-)
> >
> > --
> > 2.47.0.277.g8800431eea-goog
> >