[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4Bza5B9rSX7cw4K0iC-gW+OeEATLCcQ=6KGfmuxfJ2XOhvA@mail.gmail.com>
Date: Mon, 9 Dec 2024 15:00:36 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
Kan Liang <kan.liang@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org, Andrii Nakryiko <andrii@...nel.org>,
Song Liu <song@...nel.org>, bpf@...r.kernel.org,
Stephane Eranian <eranian@...gle.com>, Vlastimil Babka <vbabka@...e.cz>,
Roman Gushchin <roman.gushchin@...ux.dev>, Hyeonggon Yoo <42.hyeyoo@...il.com>,
Kees Cook <kees@...nel.org>
Subject: Re: [PATCH v2 2/4] perf lock contention: Run BPF slab cache iterator
On Mon, Dec 9, 2024 at 12:23 PM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> On Mon, Dec 09, 2024 at 01:38:39PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Mon, Dec 09, 2024 at 01:36:52PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Thu, Nov 07, 2024 at 10:14:57PM -0800, Namhyung Kim wrote:
> > > > Recently the kernel got the kmem_cache iterator to traverse metadata of
> > > > slab objects. This can be used to symbolize dynamic locks in a slab.
>
> > > > The new slab_caches hash map will have the pointer of the kmem_cache as
> > > > a key and save the name and a id. The id will be saved in the flags
> > > > part of the lock.
>
> > > Trying to fix this
> >
> > So you have that struct in tools/perf/util/bpf_skel/vmlinux/vmlinux.h,
> > but then, this kernel is old and doesn't have the kmem_cache iterator,
> > so using the generated vmlinux.h will fail the build.
>
> I tried passing the right offset to the iterator so as not to try to use
> a type that isn't in vmlinux.h generated from the old kernel BTF:
>
> +++ b/tools/perf/util/bpf_lock_contention.c
> @@ -52,7 +52,7 @@ static void check_slab_cache_iter(struct lock_contention *con)
> pr_debug("slab cache iterator is not available: %d\n", ret);
> goto out;
> } else {
> - const struct btf_member *s = __btf_type__find_member_by_name(btf, ret, "s");
> + const struct btf_member *s = __btf_type__find_unnamed_union_with_member_by_name(btf, ret, "s");
>
> if (s == NULL) {
> skel->rodata->slab_cache_iter_member_offset = -1;
> @@ -60,7 +60,9 @@ static void check_slab_cache_iter(struct lock_contention *con)
> goto out;
> }
>
> skel->rodata->slab_cache_iter_member_offset = s->offset / 8; // bits -> bytes
> + pr_debug("slab cache iterator kmem_cache pointer offset: %d\n",
> + skel->rodata->slab_cache_iter_member_offset);
> }
>
>
> but the verifier doesn't like that:
>
> ; struct kmem_cache *s = slab_cache_iter_member_offset < 0 ? NULL : @ lock_contention.bpf.c:615
> 12: (7b) *(u64 *)(r10 -8) = r2 ; R2_w=ctx(off=8) R10=fp0 fp-8_w=ctx(off=8)
> ; if (s == NULL) @ lock_contention.bpf.c:619
> 13: (15) if r1 == 0x0 goto pc+22 ; R1=ctx()
> ; d.id = ++slab_cache_id << LCB_F_SLAB_ID_SHIFT; @ lock_contention.bpf.c:622
> 14: (18) r1 = 0xffffc14bcde3a014 ; R1_w=map_value(map=lock_con.bss,ks=4,vs=40,off=20)
> 16: (61) r3 = *(u32 *)(r1 +0) ; R1_w=map_value(map=lock_con.bss,ks=4,vs=40,off=20) R3_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
> 17: (07) r3 += 1 ; R3_w=scalar(smin=umin=1,smax=umax=0x100000000,var_off=(0x0; 0x1ffffffff))
> 18: (63) *(u32 *)(r1 +0) = r3 ; R1_w=map_value(map=lock_con.bss,ks=4,vs=40,off=20) R3_w=scalar(smin=umin=1,smax=umax=0x100000000,var_off=(0x0; 0x1ffffffff))
> 19: (67) r3 <<= 16 ; R3_w=scalar(smin=umin=0x10000,smax=umax=0x1000000000000,smax32=0x7fff0000,umax32=0xffff0000,var_off=(0x0; 0x1ffffffff0000))
> 20: (63) *(u32 *)(r10 -40) = r3 ; R3_w=scalar(smin=umin=0x10000,smax=umax=0x1000000000000,smax32=0x7fff0000,umax32=0xffff0000,var_off=(0x0; 0x1ffffffff0000)) R10=fp0 fp-40=????scalar(smin=umin=0x10000,smax=umax=0x1000000000000,smax32=0x7fff0000,umax32=0xffff0000,var_off=(0x0; 0x1ffffffff0000))
> ; bpf_probe_read_kernel_str(d.name, sizeof(d.name), s->name); @ lock_contention.bpf.c:623
> 21: (79) r3 = *(u64 *)(r2 +96)
> dereference of modified ctx ptr R2 off=8 disallowed
> processed 19 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> -- END PROG LOAD LOG --
> libbpf: prog 'slab_cache_iter': failed to load: -EACCES
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -EACCES
> Failed to load lock-contention BPF skeleton
> lock contention BPF setup failed
> root@...ber:~#
>
> and additionally the type is not like the one you added to the barebones
> vmlinux.h:
>
> ⬢ [acme@...lbox perf-tools-next]$ git show d82e2e170d1c756b | grep 'struct bpf_iter__kmem_cache {' -A3
> +struct bpf_iter__kmem_cache {
> + struct kmem_cache *s;
> +} __attribute__((preserve_access_index));
> +
> ⬢ [acme@...lbox perf-tools-next]$
>
> But:
>
> ⬢ [acme@...lbox perf-tools-next]$ uname -a
> Linux toolbox 6.13.0-rc2 #1 SMP PREEMPT_DYNAMIC Mon Dec 9 12:33:35 -03 2024 x86_64 GNU/Linux
> ⬢ [acme@...lbox perf-tools-next]$ pahole bpf_iter__kmem_cache
> struct bpf_iter__kmem_cache {
> union {
> struct bpf_iter_meta * meta; /* 0 8 */
> }; /* 0 8 */
> union {
> struct kmem_cache * s; /* 8 8 */
> }; /* 8 8 */
>
> /* size: 16, cachelines: 1, members: 2 */
> /* last cacheline: 16 bytes */
> };
>
> ⬢ [acme@...lbox perf-tools-next]$
>
> Do CO-RE handle this?
>
I don't know exactly what the problem you are running into is, but
yes, BPF CO-RE allows handling missing fields, incompatible field type
changes, field renames, etc. All without having to break a
compilation. See [0] (and one subsection after that) for
"documentation" and examples.
[0] https://nakryiko.com/posts/bpf-core-reference-guide/#defining-own-co-re-relocatable-type-definitions
> - Arnaldo
Powered by blists - more mailing lists