Re: [PATCH 1/6] perf offcpu: Fix a build failure on old kernels

From: Arnaldo Carvalho de Melo
Date: Tue Jun 28 2022 - 10:44:56 EST


Em Fri, Jun 24, 2022 at 04:13:08PM -0700, Namhyung Kim escreveu:
> Old kernels have task_struct which contains "state" field and newer
> kernels have "__state". While the get_task_state() in the BPF code
> handles that in some way, it assumed the current kernel has the new
> definition and it caused a build error on old kernels.
>
> We should not assume anything and access them carefully. Do not use
> the task struct directly and access them using new and old definitions
> in a row.

I added a:

Fixes: edc41a1099c2d08c ("perf record: Enable off-cpu analysis with BPF")

Ok?

- Arnaldo

> 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.37.0.rc0.161.g10f37bed90-goog

--

- Arnaldo