Messages in this thread | | | From | Namhyung Kim <> | Date | Tue, 26 Sep 2023 17:50:29 -0700 | Subject | Re: [PATCH] perf record: Fix BTF type checks in the off-cpu profiling |
| |
Hi Ian,
On Sun, Sep 24, 2023 at 11:03 AM Ian Rogers <irogers@google.com> wrote: > > On Fri, Sep 22, 2023 at 4:44 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > The BTF func proto for a tracepoint has one more argument than the > > actual tracepoint function since it has a context argument at the > > begining. So it should compare to 5 when the tracepoint has 4 > > arguments. > > > > typedef void (*btf_trace_sched_switch)(void *, bool, struct task_struct *, struct task_struct *, unsigned int); > > > > Also, recent change in the perf tool would use a hand-written minimal > > vmlinux.h to generate BTF in the skeleton. So it won't have the info > > of the tracepoint. Anyway it should use the kernel's vmlinux BTF to > > check the type in the kernel. > > > > Fixes: b36888f71c85 ("perf record: Handle argument change in sched_switch") > > Cc: Song Liu <song@kernel.org> > > Cc: Hao Luo <haoluo@google.com> > > CC: bpf@vger.kernel.org > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Reviewed-by: Ian Rogers <irogers@google.com>
Thanks for the review!
> > > --- > > tools/perf/util/bpf_off_cpu.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/tools/perf/util/bpf_off_cpu.c b/tools/perf/util/bpf_off_cpu.c > > index 01f70b8e705a..21f4d9ba023d 100644 > > --- a/tools/perf/util/bpf_off_cpu.c > > +++ b/tools/perf/util/bpf_off_cpu.c > > @@ -98,7 +98,7 @@ static void off_cpu_finish(void *arg __maybe_unused) > > /* v5.18 kernel added prev_state arg, so it needs to check the signature */ > > static void check_sched_switch_args(void) > > { > > - const struct btf *btf = bpf_object__btf(skel->obj); > > + const struct btf *btf = btf__load_vmlinux_btf(); > > const struct btf_type *t1, *t2, *t3; > > u32 type_id; > > > > @@ -116,7 +116,8 @@ static void check_sched_switch_args(void) > > return; > > > > t3 = btf__type_by_id(btf, t2->type); > > - if (t3 && btf_is_func_proto(t3) && btf_vlen(t3) == 4) { > > + /* btf_trace func proto has one more argument for the context */ > > + if (t3 && btf_is_func_proto(t3) && btf_vlen(t3) == 5) { > > /* new format: pass prev_state as 4th arg */ > > nit: does this comment need updating?
No, it's the 4th arg and the sched_switch is called like.
trace_sched_switch(sched_mode & SM_MASK_PREEMPT, prev, next, prev_state);
Thanks, Namhyung
| |