| Message ID | 20220922041435.709119-1-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | perf stat: Support old kernels for bperf cgroup counting | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > The recent change in the cgroup will break the backward compatiblity in > the BPF program. It should support both old and new kernels using BPF > CO-RE technique. > > Like the task_struct->__state handling in the offcpu analysis, we can > check the field name in the cgroup struct. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > Arnaldo, I think this should go through the cgroup tree since it depends > on the earlier change there. I don't think it'd conflict with other > perf changes but please let me know if you see any trouble, thanks! FWIW, looks fine to me and I'd be happy to route this through the cgroup tree once it gets acked. Thanks.
On Fri, Sep 23, 2022 at 8:22 PM Tejun Heo <tj@kernel.org> wrote: > > On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > > The recent change in the cgroup will break the backward compatiblity in > > the BPF program. It should support both old and new kernels using BPF > > CO-RE technique. > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > check the field name in the cgroup struct. > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > Arnaldo, I think this should go through the cgroup tree since it depends > > on the earlier change there. I don't think it'd conflict with other > > perf changes but please let me know if you see any trouble, thanks! > > FWIW, looks fine to me and I'd be happy to route this through the cgroup > tree once it gets acked. Thanks Tejun! Can any perf + bpf folks take a look?
On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > The recent change in the cgroup will break the backward compatiblity in > the BPF program. It should support both old and new kernels using BPF > CO-RE technique. > > Like the task_struct->__state handling in the offcpu analysis, we can > check the field name in the cgroup struct. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > Arnaldo, I think this should go through the cgroup tree since it depends > on the earlier change there. I don't think it'd conflict with other > perf changes but please let me know if you see any trouble, thanks! could you please paste the cgroup tree link? thanks, jirka > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > index 488bd398f01d..4fe61043de04 100644 > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > @@ -43,12 +43,39 @@ struct { > __uint(value_size, sizeof(struct bpf_perf_event_value)); > } cgrp_readings SEC(".maps"); > > +/* new kernel cgroup definition */ > +struct cgroup___new { > + int level; > + struct cgroup *ancestors[]; > +} __attribute__((preserve_access_index)); > + > +/* old kernel cgroup definition */ > +struct cgroup___old { > + int level; > + u64 ancestor_ids[]; > +} __attribute__((preserve_access_index)); > + > const volatile __u32 num_events = 1; > const volatile __u32 num_cpus = 1; > > int enabled = 0; > int use_cgroup_v2 = 0; > > +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) > +{ > + /* recast pointer to capture new type for compiler */ > + struct cgroup___new *cgrp_new = (void *)cgrp; > + > + if (bpf_core_field_exists(cgrp_new->ancestors)) { > + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); > + } else { > + /* recast pointer to capture old type for compiler */ > + struct cgroup___old *cgrp_old = (void *)cgrp; > + > + return BPF_CORE_READ(cgrp_old, ancestor_ids[level]); > + } > +} > + > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > { > struct task_struct *p = (void *)bpf_get_current_task(); > @@ -70,7 +97,7 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > break; > > // convert cgroup-id to a map index > - cgrp_id = BPF_CORE_READ(cgrp, ancestors[i], kn, id); > + cgrp_id = get_cgroup_v1_ancestor_id(cgrp, i); > elem = bpf_map_lookup_elem(&cgrp_idx, &cgrp_id); > if (!elem) > continue; > -- > 2.37.3.968.ga6b4b080e4-goog >
Hi Jiri, On Fri, Sep 30, 2022 at 2:44 PM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > > The recent change in the cgroup will break the backward compatiblity in > > the BPF program. It should support both old and new kernels using BPF > > CO-RE technique. > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > check the field name in the cgroup struct. > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > Arnaldo, I think this should go through the cgroup tree since it depends > > on the earlier change there. I don't think it'd conflict with other > > perf changes but please let me know if you see any trouble, thanks! > > could you please paste the cgroup tree link? Do you mean this? https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git Thanks,. Namhyung
On September 30, 2022 6:56:40 PM GMT-03:00, Namhyung Kim <namhyung@kernel.org> wrote: >Hi Jiri, > >On Fri, Sep 30, 2022 at 2:44 PM Jiri Olsa <olsajiri@gmail.com> wrote: >> >> On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: >> > The recent change in the cgroup will break the backward compatiblity in >> > the BPF program. It should support both old and new kernels using BPF >> > CO-RE technique. >> > >> > Like the task_struct->__state handling in the offcpu analysis, we can >> > check the field name in the cgroup struct. >> > >> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> >> > --- >> > Arnaldo, I think this should go through the cgroup tree since it depends >> > on the earlier change there. I don't think it'd conflict with other >> > perf changes but please let me know if you see any trouble, thanks! >> >> could you please paste the cgroup tree link? > >Do you mean this? > > https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git > Which branch and cset in that tree does you perf skel depends on? - Arnaldo >Thanks,. >Namhyung
On Fri, Sep 30, 2022 at 3:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > > > On September 30, 2022 6:56:40 PM GMT-03:00, Namhyung Kim <namhyung@kernel.org> wrote: > >Hi Jiri, > > > >On Fri, Sep 30, 2022 at 2:44 PM Jiri Olsa <olsajiri@gmail.com> wrote: > >> > >> On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > >> > The recent change in the cgroup will break the backward compatiblity in > >> > the BPF program. It should support both old and new kernels using BPF > >> > CO-RE technique. > >> > > >> > Like the task_struct->__state handling in the offcpu analysis, we can > >> > check the field name in the cgroup struct. > >> > > >> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > >> > --- > >> > Arnaldo, I think this should go through the cgroup tree since it depends > >> > on the earlier change there. I don't think it'd conflict with other > >> > perf changes but please let me know if you see any trouble, thanks! > >> > >> could you please paste the cgroup tree link? > > > >Do you mean this? > > > > https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git > > > > > Which branch and cset in that tree does you perf skel depends on? I believe it's for-6.1 and the cset is in https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git/commit/?h=for-6.1&id=7f203bc89eb66d6afde7eae91347fc0352090cc3 Thanks, Namhyung
On Wed, Sep 21, 2022 at 9:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > The recent change in the cgroup will break the backward compatiblity in > the BPF program. It should support both old and new kernels using BPF > CO-RE technique. > > Like the task_struct->__state handling in the offcpu analysis, we can > check the field name in the cgroup struct. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > Arnaldo, I think this should go through the cgroup tree since it depends > on the earlier change there. I don't think it'd conflict with other > perf changes but please let me know if you see any trouble, thanks! > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > index 488bd398f01d..4fe61043de04 100644 > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > @@ -43,12 +43,39 @@ struct { > __uint(value_size, sizeof(struct bpf_perf_event_value)); > } cgrp_readings SEC(".maps"); > > +/* new kernel cgroup definition */ > +struct cgroup___new { > + int level; > + struct cgroup *ancestors[]; > +} __attribute__((preserve_access_index)); > + > +/* old kernel cgroup definition */ > +struct cgroup___old { > + int level; > + u64 ancestor_ids[]; > +} __attribute__((preserve_access_index)); > + > const volatile __u32 num_events = 1; > const volatile __u32 num_cpus = 1; > > int enabled = 0; > int use_cgroup_v2 = 0; > > +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) > +{ > + /* recast pointer to capture new type for compiler */ > + struct cgroup___new *cgrp_new = (void *)cgrp; > + > + if (bpf_core_field_exists(cgrp_new->ancestors)) { > + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); have you checked generated BPF code for this ancestors[level] access? I'd expect CO-RE relocation for finding ancestors offset and then just normal + level * 8 arithmetic, but would be nice to confirm. Apart from this, looks good to me: Acked-by: Andrii Nakryiko <andrii@kernel.org> > + } else { > + /* recast pointer to capture old type for compiler */ > + struct cgroup___old *cgrp_old = (void *)cgrp; > + > + return BPF_CORE_READ(cgrp_old, ancestor_ids[level]); > + } > +} > + > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > { > struct task_struct *p = (void *)bpf_get_current_task(); > @@ -70,7 +97,7 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > break; > > // convert cgroup-id to a map index > - cgrp_id = BPF_CORE_READ(cgrp, ancestors[i], kn, id); > + cgrp_id = get_cgroup_v1_ancestor_id(cgrp, i); > elem = bpf_map_lookup_elem(&cgrp_idx, &cgrp_id); > if (!elem) > continue; > -- > 2.37.3.968.ga6b4b080e4-goog >
Hello, On Fri, Sep 30, 2022 at 3:48 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Sep 21, 2022 at 9:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > The recent change in the cgroup will break the backward compatiblity in > > the BPF program. It should support both old and new kernels using BPF > > CO-RE technique. > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > check the field name in the cgroup struct. > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > Arnaldo, I think this should go through the cgroup tree since it depends > > on the earlier change there. I don't think it'd conflict with other > > perf changes but please let me know if you see any trouble, thanks! > > > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > index 488bd398f01d..4fe61043de04 100644 > > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > @@ -43,12 +43,39 @@ struct { > > __uint(value_size, sizeof(struct bpf_perf_event_value)); > > } cgrp_readings SEC(".maps"); > > > > +/* new kernel cgroup definition */ > > +struct cgroup___new { > > + int level; > > + struct cgroup *ancestors[]; > > +} __attribute__((preserve_access_index)); > > + > > +/* old kernel cgroup definition */ > > +struct cgroup___old { > > + int level; > > + u64 ancestor_ids[]; > > +} __attribute__((preserve_access_index)); > > + > > const volatile __u32 num_events = 1; > > const volatile __u32 num_cpus = 1; > > > > int enabled = 0; > > int use_cgroup_v2 = 0; > > > > +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) > > +{ > > + /* recast pointer to capture new type for compiler */ > > + struct cgroup___new *cgrp_new = (void *)cgrp; > > + > > + if (bpf_core_field_exists(cgrp_new->ancestors)) { > > + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); > > have you checked generated BPF code for this ancestors[level] access? > I'd expect CO-RE relocation for finding ancestors offset and then just > normal + level * 8 arithmetic, but would be nice to confirm. Apart > from this, looks good to me: > > Acked-by: Andrii Nakryiko <andrii@kernel.org> Thanks for your review! How can I check the generated code? Do you have something works with skeletons or do I have to save the BPF object somehow during the build? Thanks, Namhyung
On Fri, Sep 30, 2022 at 02:56:40PM -0700, Namhyung Kim wrote: > Hi Jiri, > > On Fri, Sep 30, 2022 at 2:44 PM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > > > The recent change in the cgroup will break the backward compatiblity in > > > the BPF program. It should support both old and new kernels using BPF > > > CO-RE technique. > > > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > > check the field name in the cgroup struct. > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > --- > > > Arnaldo, I think this should go through the cgroup tree since it depends > > > on the earlier change there. I don't think it'd conflict with other > > > perf changes but please let me know if you see any trouble, thanks! > > > > could you please paste the cgroup tree link? > > Do you mean this? > > https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git yea, wanted to try that.. I cherry-picked the 7f203bc89eb6 ;-) thanks, jirka > > Thanks,. > Namhyung
On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > The recent change in the cgroup will break the backward compatiblity in > the BPF program. It should support both old and new kernels using BPF > CO-RE technique. > > Like the task_struct->__state handling in the offcpu analysis, we can > check the field name in the cgroup struct. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> lgtm Acked-by: Jiri Olsa <jolsa@kernel.org> jirka > --- > Arnaldo, I think this should go through the cgroup tree since it depends > on the earlier change there. I don't think it'd conflict with other > perf changes but please let me know if you see any trouble, thanks! > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > index 488bd398f01d..4fe61043de04 100644 > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > @@ -43,12 +43,39 @@ struct { > __uint(value_size, sizeof(struct bpf_perf_event_value)); > } cgrp_readings SEC(".maps"); > > +/* new kernel cgroup definition */ > +struct cgroup___new { > + int level; > + struct cgroup *ancestors[]; > +} __attribute__((preserve_access_index)); > + > +/* old kernel cgroup definition */ > +struct cgroup___old { > + int level; > + u64 ancestor_ids[]; > +} __attribute__((preserve_access_index)); > + > const volatile __u32 num_events = 1; > const volatile __u32 num_cpus = 1; > > int enabled = 0; > int use_cgroup_v2 = 0; > > +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) > +{ > + /* recast pointer to capture new type for compiler */ > + struct cgroup___new *cgrp_new = (void *)cgrp; > + > + if (bpf_core_field_exists(cgrp_new->ancestors)) { > + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); > + } else { > + /* recast pointer to capture old type for compiler */ > + struct cgroup___old *cgrp_old = (void *)cgrp; > + > + return BPF_CORE_READ(cgrp_old, ancestor_ids[level]); > + } > +} > + > static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > { > struct task_struct *p = (void *)bpf_get_current_task(); > @@ -70,7 +97,7 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) > break; > > // convert cgroup-id to a map index > - cgrp_id = BPF_CORE_READ(cgrp, ancestors[i], kn, id); > + cgrp_id = get_cgroup_v1_ancestor_id(cgrp, i); > elem = bpf_map_lookup_elem(&cgrp_idx, &cgrp_id); > if (!elem) > continue; > -- > 2.37.3.968.ga6b4b080e4-goog >
On Fri, Sep 30, 2022 at 7:31 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Hello, > > On Fri, Sep 30, 2022 at 3:48 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, Sep 21, 2022 at 9:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > The recent change in the cgroup will break the backward compatiblity in > > > the BPF program. It should support both old and new kernels using BPF > > > CO-RE technique. > > > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > > check the field name in the cgroup struct. > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > --- > > > Arnaldo, I think this should go through the cgroup tree since it depends > > > on the earlier change there. I don't think it'd conflict with other > > > perf changes but please let me know if you see any trouble, thanks! > > > > > > tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- > > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > > index 488bd398f01d..4fe61043de04 100644 > > > --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > > +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c > > > @@ -43,12 +43,39 @@ struct { > > > __uint(value_size, sizeof(struct bpf_perf_event_value)); > > > } cgrp_readings SEC(".maps"); > > > > > > +/* new kernel cgroup definition */ > > > +struct cgroup___new { > > > + int level; > > > + struct cgroup *ancestors[]; > > > +} __attribute__((preserve_access_index)); > > > + > > > +/* old kernel cgroup definition */ > > > +struct cgroup___old { > > > + int level; > > > + u64 ancestor_ids[]; > > > +} __attribute__((preserve_access_index)); > > > + > > > const volatile __u32 num_events = 1; > > > const volatile __u32 num_cpus = 1; > > > > > > int enabled = 0; > > > int use_cgroup_v2 = 0; > > > > > > +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) > > > +{ > > > + /* recast pointer to capture new type for compiler */ > > > + struct cgroup___new *cgrp_new = (void *)cgrp; > > > + > > > + if (bpf_core_field_exists(cgrp_new->ancestors)) { > > > + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); > > > > have you checked generated BPF code for this ancestors[level] access? > > I'd expect CO-RE relocation for finding ancestors offset and then just > > normal + level * 8 arithmetic, but would be nice to confirm. Apart > > from this, looks good to me: > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > Thanks for your review! > > How can I check the generated code? Do you have something works with > skeletons or do I have to save the BPF object somehow during the build? > skeleton is generated from ELF BPF object file. You can do llvm-objdump -d <obj.bpf.o> to see instructions. Unfortunately you can't see BPF CO-RE relocations this way, you'd have to use something like my custom tool ([0]). But anyways, I checked locally similar code pattern and I think it's all good from BPF CO-RE perspective. I see appropriate relocations in all the necessary places. So this should work. Acked-by: Andrii Nakryiko <andrii@kernel.org> [0] https://github.com/anakryiko/btfdump > Thanks, > Namhyung
On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > The recent change in the cgroup will break the backward compatiblity in > the BPF program. It should support both old and new kernels using BPF > CO-RE technique. > > Like the task_struct->__state handling in the offcpu analysis, we can > check the field name in the cgroup struct. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Looks like it's acked enough but the patch doesn't apply anymore. Namhyung, can you please refresh the patch? I'll route this through cgroup/for-6.1-fixes unless somebody objects. Thanks.
Hi Tejun, On Mon, Oct 10, 2022 at 4:59 PM Tejun Heo <tj@kernel.org> wrote: > > On Wed, Sep 21, 2022 at 09:14:35PM -0700, Namhyung Kim wrote: > > The recent change in the cgroup will break the backward compatiblity in > > the BPF program. It should support both old and new kernels using BPF > > CO-RE technique. > > > > Like the task_struct->__state handling in the offcpu analysis, we can > > check the field name in the cgroup struct. > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Looks like it's acked enough but the patch doesn't apply anymore. Namhyung, > can you please refresh the patch? I'll route this through > cgroup/for-6.1-fixes unless somebody objects. Sorry about the conflict. There was another change to get the perf cgroup subsys id properly. Will send v2 soon. Thanks, Namhyung
diff --git a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c index 488bd398f01d..4fe61043de04 100644 --- a/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c +++ b/tools/perf/util/bpf_skel/bperf_cgroup.bpf.c @@ -43,12 +43,39 @@ struct { __uint(value_size, sizeof(struct bpf_perf_event_value)); } cgrp_readings SEC(".maps"); +/* new kernel cgroup definition */ +struct cgroup___new { + int level; + struct cgroup *ancestors[]; +} __attribute__((preserve_access_index)); + +/* old kernel cgroup definition */ +struct cgroup___old { + int level; + u64 ancestor_ids[]; +} __attribute__((preserve_access_index)); + const volatile __u32 num_events = 1; const volatile __u32 num_cpus = 1; int enabled = 0; int use_cgroup_v2 = 0; +static inline __u64 get_cgroup_v1_ancestor_id(struct cgroup *cgrp, int level) +{ + /* recast pointer to capture new type for compiler */ + struct cgroup___new *cgrp_new = (void *)cgrp; + + if (bpf_core_field_exists(cgrp_new->ancestors)) { + return BPF_CORE_READ(cgrp_new, ancestors[level], kn, id); + } else { + /* recast pointer to capture old type for compiler */ + struct cgroup___old *cgrp_old = (void *)cgrp; + + return BPF_CORE_READ(cgrp_old, ancestor_ids[level]); + } +} + static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) { struct task_struct *p = (void *)bpf_get_current_task(); @@ -70,7 +97,7 @@ static inline int get_cgroup_v1_idx(__u32 *cgrps, int size) break; // convert cgroup-id to a map index - cgrp_id = BPF_CORE_READ(cgrp, ancestors[i], kn, id); + cgrp_id = get_cgroup_v1_ancestor_id(cgrp, i); elem = bpf_map_lookup_elem(&cgrp_idx, &cgrp_id); if (!elem) continue;
The recent change in the cgroup will break the backward compatiblity in the BPF program. It should support both old and new kernels using BPF CO-RE technique. Like the task_struct->__state handling in the offcpu analysis, we can check the field name in the cgroup struct. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- Arnaldo, I think this should go through the cgroup tree since it depends on the earlier change there. I don't think it'd conflict with other perf changes but please let me know if you see any trouble, thanks! tools/perf/util/bpf_skel/bperf_cgroup.bpf.c | 29 ++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)