| Message ID | 20220923063205.772936-1-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | [v4] perf tools: Get a perf cgroup more portably in BPF | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-15 | success | Logs for test_verifier on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-16 | success | Logs for test_verifier on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-17 | success | Logs for test_verifier on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-9 | success | Logs for test_progs on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-12 | success | Logs for test_progs_no_alu32 on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-13 | success | Logs for test_progs_no_alu32 on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-14 | success | Logs for test_progs_no_alu32 on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-10 | success | Logs for test_progs on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-11 | success | Logs for test_progs on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-7 | success | Logs for test_maps on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-8 | success | Logs for test_maps on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for set-matrix |
| bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for llvm-toolchain |
| bpf/vmtest-bpf-next-VM_Test-5 | success | Logs for set-matrix |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for build for x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for build for x86_64 with llvm-16 |
On Thu, Sep 22, 2022 at 11:32 PM Namhyung Kim <namhyung@kernel.org> wrote: > > The perf_event_cgrp_id can be different on other configurations. > To be more portable as CO-RE, it needs to get the cgroup subsys id > using the bpf_core_enum_value() helper. > > Suggested-by: Ian Rogers <irogers@google.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Reviewed-by: Ian Rogers <irogers@google.com> Would be good to get this into perf/urgent, does it need Fixes tags for that? Thanks, Ian > --- > v4 changes) > * add a missing check in the off_cpu > > v3 changes) > * check compiler features for enum value > > v2 changes) > * fix off_cpu.bpf.c too > * get perf_subsys_id only once > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 11 ++++++++++- > tools/perf/util/bpf_skel/off_cpu.bpf.c | 18 ++++++++++++++---- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > index 292c430768b5..8e7520e273db 100644 > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > @@ -48,6 +48,7 @@ const volatile __u32 num_cpus = 1; > > int enabled = 0; > int use_cgroup_v2 = 0; > +int perf_subsys_id = -1; > > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > { > @@ -58,7 +59,15 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > int level; > int cnt; > > - cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_event_cgrp_id], cgroup); > + if (perf_subsys_id == -1) { > +#if __has_builtin(__builtin_preserve_enum_value) > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > + perf_event_cgrp_id); > +#else > + perf_subsys_id = perf_event_cgrp_id; > +#endif > + } > + cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_subsys_id], cgroup); > level = BPF_CORE_READ(cgrp, level); > > for (cnt = 0; i < MAX_LEVELS; i++) { > diff --git a/tools/perf/util/bpf_skel/off_cpu.bpf.c b/tools/perf/util/bpf_skel/off_cpu.bpf.c > index c4ba2bcf179f..38e3b287dbb2 100644 > --- a/tools/perf/util/bpf_skel/off_cpu.bpf.c > +++ b/tools/perf/util/bpf_skel/off_cpu.bpf.c > @@ -94,6 +94,8 @@ const volatile bool has_prev_state = false; > const volatile bool needs_cgroup = false; > const volatile bool uses_cgroup_v1 = false; > > +int perf_subsys_id = -1; > + > /* > * 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: > @@ -119,11 +121,19 @@ static inline __u64 get_cgroup_id(struct task_struct *t) > { > struct cgroup *cgrp; > > - if (uses_cgroup_v1) > - cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_event_cgrp_id], cgroup); > - else > - cgrp = BPF_CORE_READ(t, cgroups, dfl_cgrp); > + if (!uses_cgroup_v1) > + return BPF_CORE_READ(t, cgroups, dfl_cgrp, kn, id); > + > + if (perf_subsys_id == -1) { > +#if __has_builtin(__builtin_preserve_enum_value) > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > + perf_event_cgrp_id); > +#else > + perf_subsys_id = perf_event_cgrp_id; > +#endif > + } > > + cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_subsys_id], cgroup); > return BPF_CORE_READ(cgrp, kn, id); > } > > -- > 2.37.3.998.g577e59143f-goog >
Em Thu, Sep 22, 2022 at 11:32:05PM -0700, Namhyung Kim escreveu: > The perf_event_cgrp_id can be different on other configurations. > To be more portable as CO-RE, it needs to get the cgroup subsys id > using the bpf_core_enum_value() helper. > > Suggested-by: Ian Rogers <irogers@google.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > v4 changes) > * add a missing check in the off_cpu Thanks, applied. - Arnaldo > v3 changes) > * check compiler features for enum value > > v2 changes) > * fix off_cpu.bpf.c too > * get perf_subsys_id only once > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 11 ++++++++++- > tools/perf/util/bpf_skel/off_cpu.bpf.c | 18 ++++++++++++++---- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > index 292c430768b5..8e7520e273db 100644 > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > @@ -48,6 +48,7 @@ const volatile __u32 num_cpus = 1; > > int enabled = 0; > int use_cgroup_v2 = 0; > +int perf_subsys_id = -1; > > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > { > @@ -58,7 +59,15 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > int level; > int cnt; > > - cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_event_cgrp_id], cgroup); > + if (perf_subsys_id == -1) { > +#if __has_builtin(__builtin_preserve_enum_value) > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > + perf_event_cgrp_id); > +#else > + perf_subsys_id = perf_event_cgrp_id; > +#endif > + } > + cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_subsys_id], cgroup); > level = BPF_CORE_READ(cgrp, level); > > for (cnt = 0; i < MAX_LEVELS; i++) { > diff --git a/tools/perf/util/bpf_skel/off_cpu.bpf.c b/tools/perf/util/bpf_skel/off_cpu.bpf.c > index c4ba2bcf179f..38e3b287dbb2 100644 > --- a/tools/perf/util/bpf_skel/off_cpu.bpf.c > +++ b/tools/perf/util/bpf_skel/off_cpu.bpf.c > @@ -94,6 +94,8 @@ const volatile bool has_prev_state = false; > const volatile bool needs_cgroup = false; > const volatile bool uses_cgroup_v1 = false; > > +int perf_subsys_id = -1; > + > /* > * 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: > @@ -119,11 +121,19 @@ static inline __u64 get_cgroup_id(struct task_struct *t) > { > struct cgroup *cgrp; > > - if (uses_cgroup_v1) > - cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_event_cgrp_id], cgroup); > - else > - cgrp = BPF_CORE_READ(t, cgroups, dfl_cgrp); > + if (!uses_cgroup_v1) > + return BPF_CORE_READ(t, cgroups, dfl_cgrp, kn, id); > + > + if (perf_subsys_id == -1) { > +#if __has_builtin(__builtin_preserve_enum_value) > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > + perf_event_cgrp_id); > +#else > + perf_subsys_id = perf_event_cgrp_id; > +#endif > + } > > + cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_subsys_id], cgroup); > return BPF_CORE_READ(cgrp, kn, id); > } > > -- > 2.37.3.998.g577e59143f-goog
Em Fri, Sep 23, 2022 at 09:45:19AM -0700, Ian Rogers escreveu: > On Thu, Sep 22, 2022 at 11:32 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > The perf_event_cgrp_id can be different on other configurations. > > To be more portable as CO-RE, it needs to get the cgroup subsys id > > using the bpf_core_enum_value() helper. > > > > Suggested-by: Ian Rogers <irogers@google.com> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Reviewed-by: Ian Rogers <irogers@google.com> > > Would be good to get this into perf/urgent, does it need Fixes tags for that? I got it into the perf/urgent branch. - Arnaldo > Thanks, > Ian > > > --- > > v4 changes) > > * add a missing check in the off_cpu > > > > v3 changes) > > * check compiler features for enum value > > > > v2 changes) > > * fix off_cpu.bpf.c too > > * get perf_subsys_id only once > > > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 11 ++++++++++- > > tools/perf/util/bpf_skel/off_cpu.bpf.c | 18 ++++++++++++++---- > > 2 files changed, 24 insertions(+), 5 deletions(-) > > > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > index 292c430768b5..8e7520e273db 100644 > > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > @@ -48,6 +48,7 @@ const volatile __u32 num_cpus = 1; > > > > int enabled = 0; > > int use_cgroup_v2 = 0; > > +int perf_subsys_id = -1; > > > > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > > { > > @@ -58,7 +59,15 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > > int level; > > int cnt; > > > > - cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_event_cgrp_id], cgroup); > > + if (perf_subsys_id == -1) { > > +#if __has_builtin(__builtin_preserve_enum_value) > > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > > + perf_event_cgrp_id); > > +#else > > + perf_subsys_id = perf_event_cgrp_id; > > +#endif > > + } > > + cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_subsys_id], cgroup); > > level = BPF_CORE_READ(cgrp, level); > > > > for (cnt = 0; i < MAX_LEVELS; i++) { > > diff --git a/tools/perf/util/bpf_skel/off_cpu.bpf.c b/tools/perf/util/bpf_skel/off_cpu.bpf.c > > index c4ba2bcf179f..38e3b287dbb2 100644 > > --- a/tools/perf/util/bpf_skel/off_cpu.bpf.c > > +++ b/tools/perf/util/bpf_skel/off_cpu.bpf.c > > @@ -94,6 +94,8 @@ const volatile bool has_prev_state = false; > > const volatile bool needs_cgroup = false; > > const volatile bool uses_cgroup_v1 = false; > > > > +int perf_subsys_id = -1; > > + > > /* > > * 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: > > @@ -119,11 +121,19 @@ static inline __u64 get_cgroup_id(struct task_struct *t) > > { > > struct cgroup *cgrp; > > > > - if (uses_cgroup_v1) > > - cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_event_cgrp_id], cgroup); > > - else > > - cgrp = BPF_CORE_READ(t, cgroups, dfl_cgrp); > > + if (!uses_cgroup_v1) > > + return BPF_CORE_READ(t, cgroups, dfl_cgrp, kn, id); > > + > > + if (perf_subsys_id == -1) { > > +#if __has_builtin(__builtin_preserve_enum_value) > > + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, > > + perf_event_cgrp_id); > > +#else > > + perf_subsys_id = perf_event_cgrp_id; > > +#endif > > + } > > > > + cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_subsys_id], cgroup); > > return BPF_CORE_READ(cgrp, kn, id); > > } > > > > -- > > 2.37.3.998.g577e59143f-goog > >
diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c index 292c430768b5..8e7520e273db 100644 --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c @@ -48,6 +48,7 @@ const volatile __u32 num_cpus = 1; int enabled = 0; int use_cgroup_v2 = 0; +int perf_subsys_id = -1; static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) { @@ -58,7 +59,15 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) int level; int cnt; - cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_event_cgrp_id], cgroup); + if (perf_subsys_id == -1) { +#if __has_builtin(__builtin_preserve_enum_value) + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, + perf_event_cgrp_id); +#else + perf_subsys_id = perf_event_cgrp_id; +#endif + } + cgrp = BPF_CORE_READ(p, cgroups, subsys[perf_subsys_id], cgroup); level = BPF_CORE_READ(cgrp, level); for (cnt = 0; i < MAX_LEVELS; i++) { diff --git a/tools/perf/util/bpf_skel/off_cpu.bpf.c b/tools/perf/util/bpf_skel/off_cpu.bpf.c index c4ba2bcf179f..38e3b287dbb2 100644 --- a/tools/perf/util/bpf_skel/off_cpu.bpf.c +++ b/tools/perf/util/bpf_skel/off_cpu.bpf.c @@ -94,6 +94,8 @@ const volatile bool has_prev_state = false; const volatile bool needs_cgroup = false; const volatile bool uses_cgroup_v1 = false; +int perf_subsys_id = -1; + /* * 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: @@ -119,11 +121,19 @@ static inline __u64 get_cgroup_id(struct task_struct *t) { struct cgroup *cgrp; - if (uses_cgroup_v1) - cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_event_cgrp_id], cgroup); - else - cgrp = BPF_CORE_READ(t, cgroups, dfl_cgrp); + if (!uses_cgroup_v1) + return BPF_CORE_READ(t, cgroups, dfl_cgrp, kn, id); + + if (perf_subsys_id == -1) { +#if __has_builtin(__builtin_preserve_enum_value) + perf_subsys_id = bpf_core_enum_value(enum cgroup_subsys_id, + perf_event_cgrp_id); +#else + perf_subsys_id = perf_event_cgrp_id; +#endif + } + cgrp = BPF_CORE_READ(t, cgroups, subsys[perf_subsys_id], cgroup); return BPF_CORE_READ(cgrp, kn, id); }
The perf_event_cgrp_id can be different on other configurations. To be more portable as CO-RE, it needs to get the cgroup subsys id using the bpf_core_enum_value() helper. Suggested-by: Ian Rogers <irogers@google.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- v4 changes) * add a missing check in the off_cpu v3 changes) * check compiler features for enum value v2 changes) * fix off_cpu.bpf.c too * get perf_subsys_id only once tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 11 ++++++++++- tools/perf/util/bpf_skel/off_cpu.bpf.c | 18 ++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-)