On Tue, May 31, 2022 at 11:01:14PM -0700, Namhyung Kim wrote: > On Tue, May 31, 2022 at 5:00 PM Ian Rogers <irogers@xxxxxxxxxx> wrote: > > > > On Wed, May 18, 2022 at 3:47 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote: > [SNIP] > > > + > > > +/* > > > + * Old kernel used to call it task_struct->state and now it's '__state'. > > > + * Use BPF CO-RE "ignored suffix rule" to deal with it like below: > > > + * > > > + * https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes > > > + */ > > > +static inline int get_task_state(struct task_struct *t) > > > +{ > > > + if (bpf_core_field_exists(t->__state)) > > > + return BPF_CORE_READ(t, __state); > > > + > > > > When building against a pre-5.14 kernel I'm running into a build issue here: > > > > tools/perf/util/bpf_skel/off_cpu.bpf.c:96:31: error: no member named '__ > > state' in 'struct task_struct'; did you mean 'state'? > > if (bpf_core_field_exists(t->__state)) > > ^~~~~~~ > > state > > > > This isn't covered by Andrii's BPF CO-RE reference guide. I have an > > #iffy workaround below,but this will be brittle if the 5.14+ kernel > > code is backported. Suggestions welcomed :-) > > Thanks for the fix. I think we should not guess the field name > in the current task struct and check both versions separately. > I'm afraid the version check won't work with some backported > kernels. But do we care? What about this instead? ----8<---- >From a621f836f00e11942e5d39a735ec8f7a21962d6a Mon Sep 17 00:00:00 2001 From: Namhyung Kim <namhyung@xxxxxxxxxx> Date: Tue, 31 May 2022 14:25:05 -0700 Subject: [PATCH] perf tools: Fix a build failure in off-cpu BPF program on old kernels Old kernels have task_struct which contains "state" field. While the get_task_state() in the BPF code handles that, it assumed the kernel has the new definition and caused a build error on old kernels. We should not assume anything and access them carefully. Reported-by: Ian Rogers <irogers@xxxxxxxxxx> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx> --- tools/perf/util/bpf_skel/off_cpu.bpf.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/bpf_skel/off_cpu.bpf.c b/tools/perf/util/bpf_skel/off_cpu.bpf.c index 792ae2847080..cc6d7fd55118 100644 --- a/tools/perf/util/bpf_skel/off_cpu.bpf.c +++ b/tools/perf/util/bpf_skel/off_cpu.bpf.c @@ -71,6 +71,11 @@ struct { __uint(max_entries, 1); } cgroup_filter SEC(".maps"); +/* new kernel task_struct definition */ +struct task_struct___new { + long __state; +} __attribute__((preserve_access_index)); + /* old kernel task_struct definition */ struct task_struct___old { long state; @@ -93,14 +98,17 @@ const volatile bool uses_cgroup_v1 = false; */ static inline int get_task_state(struct task_struct *t) { - if (bpf_core_field_exists(t->__state)) - return BPF_CORE_READ(t, __state); + /* recast pointer to capture new type for compiler */ + struct task_struct___new *t_new = (void *)t; - /* recast pointer to capture task_struct___old type for compiler */ - struct task_struct___old *t_old = (void *)t; + if (bpf_core_field_exists(t_new->__state)) { + return BPF_CORE_READ(t_new, __state); + } else { + /* recast pointer to capture old type for compiler */ + struct task_struct___old *t_old = (void *)t; - /* now use old "state" name of the field */ - return BPF_CORE_READ(t_old, state); + return BPF_CORE_READ(t_old, state); + } } static inline __u64 get_cgroup_id(struct task_struct *t) -- 2.36.1.255.ge46751e96f-goog
