| Message ID | 20220520012133.1217211-3-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: rstat: cgroup hierarchical stats | expand |
On Fri, May 20, 2022 at 01:21:30AM +0000, Yosry Ahmed wrote: > Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > tracing programs. bpf programs that make use of rstat can use these > functions to inform rstat when they update stats for a cgroup, and when > they need to flush the stats. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Do patch 1 and 2 need to be separate? Also, can you explain and comment why it's __weak? Thanks.
On Fri, May 20, 2022 at 12:24 AM Tejun Heo <tj@kernel.org> wrote: > > On Fri, May 20, 2022 at 01:21:30AM +0000, Yosry Ahmed wrote: > > Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > > tracing programs. bpf programs that make use of rstat can use these > > functions to inform rstat when they update stats for a cgroup, and when > > they need to flush the stats. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > Do patch 1 and 2 need to be separate? Also, can you explain and comment why > it's __weak? I will squash them in the next version. As for the declaration, I took the __weak annotation from Alexei's reply to the previous version. I thought it had something to do with how fentry progs attach to functions with BTF and all. When I try the same code with a static noinline declaration instead, fentry attachment fails to find the BTF type ID of bpf_rstat_flush. When I try it with just noinline (without __weak), the fentry program attaches, but is never invoked. I tried looking at the attach code but I couldn't figure out why this happens. In retrospect, I should have given this more thought. It would be great if Alexei could shed some light on this. > > Thanks. > > -- > tejun
On Fri, May 20, 2022 at 02:43:03PM IST, Yosry Ahmed wrote: > On Fri, May 20, 2022 at 12:24 AM Tejun Heo <tj@kernel.org> wrote: > > > > On Fri, May 20, 2022 at 01:21:30AM +0000, Yosry Ahmed wrote: > > > Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > > > tracing programs. bpf programs that make use of rstat can use these > > > functions to inform rstat when they update stats for a cgroup, and when > > > they need to flush the stats. > > > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > > > Do patch 1 and 2 need to be separate? Also, can you explain and comment why > > it's __weak? > > I will squash them in the next version. > > As for the declaration, I took the __weak annotation from Alexei's > reply to the previous version. I thought it had something to do with > how fentry progs attach to functions with BTF and all. > When I try the same code with a static noinline declaration instead, > fentry attachment fails to find the BTF type ID of bpf_rstat_flush. > When I try it with just noinline (without __weak), the fentry program > attaches, but is never invoked. I tried looking at the attach code but > I couldn't figure out why this happens. > With static noinline, the compiler will optimize away the function. With global noinline, it can still optimize away the call site, but will keep the function definition, so attach works. Therefore __weak is needed to ensure call is still emitted. With GCC __attribute__((noipa)) might have been more appropritate, but LLVM doesn't support it, so __weak is the next best thing supported by both with the same side effect. > In retrospect, I should have given this more thought. It would be > great if Alexei could shed some light on this. > > > > > Thanks. > > > > > > -- > > tejun -- Kartikeya
On Fri, May 20, 2022 at 03:06:07PM +0530, Kumar Kartikeya Dwivedi wrote: > With static noinline, the compiler will optimize away the function. With global > noinline, it can still optimize away the call site, but will keep the function > definition, so attach works. Therefore __weak is needed to ensure call is still > emitted. With GCC __attribute__((noipa)) might have been more appropritate, but > LLVM doesn't support it, so __weak is the next best thing supported by both with > the same side effect. Ah, okay, so it's to prevent compiler from optimizing away call to a noop function by telling it that we don't know what the function might eventually be. Thanks for the explanation. Yosry, can you please add a comment explaining what's going on? Thanks.
On 5/19/22 6:21 PM, Yosry Ahmed wrote: > Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > tracing programs. bpf programs that make use of rstat can use these > functions to inform rstat when they update stats for a cgroup, and when > they need to flush the stats. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > --- > kernel/cgroup/rstat.c | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > index e7a88d2600bd..a16a851bc0a1 100644 > --- a/kernel/cgroup/rstat.c > +++ b/kernel/cgroup/rstat.c > @@ -3,6 +3,11 @@ > > #include <linux/sched/cputime.h> > > +#include <linux/bpf.h> > +#include <linux/btf.h> > +#include <linux/btf_ids.h> > + > + > static DEFINE_SPINLOCK(cgroup_rstat_lock); > static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); > > @@ -141,7 +146,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos, > return pos; > } > > -/* A hook for bpf stat collectors to attach to and flush their stats */ > +/* > + * A hook for bpf stat collectors to attach to and flush their stats. > + * Together with providing bpf kfuncs for cgroup_rstat_updated() and > + * cgroup_rstat_flush(), this enables a complete workflow where bpf progs that > + * collect cgroup stats can integrate with rstat for efficient flushing. > + */ > __weak noinline void bpf_rstat_flush(struct cgroup *cgrp, > struct cgroup *parent, int cpu) > { > @@ -476,3 +486,26 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq) > "system_usec %llu\n", > usage, utime, stime); > } > + > +/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */ > +BTF_SET_START(bpf_rstat_check_kfunc_ids) > +BTF_ID(func, cgroup_rstat_updated) > +BTF_ID(func, cgroup_rstat_flush) > +BTF_SET_END(bpf_rstat_check_kfunc_ids) > + > +BTF_SET_START(bpf_rstat_sleepable_kfunc_ids) > +BTF_ID(func, cgroup_rstat_flush) > +BTF_SET_END(bpf_rstat_sleepable_kfunc_ids) > + > +static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = { > + .owner = THIS_MODULE, > + .check_set = &bpf_rstat_check_kfunc_ids, > + .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, There is a compilation error here: kernel/cgroup/rstat.c:503:3: error: ‘const struct btf_kfunc_id_set’ has no member named ‘sleepable_set’; did you mean ‘release_set’? 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, | ^~~~~~~~~~~~~ | release_set kernel/cgroup/rstat.c:503:19: warning: excess elements in struct initializer 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, | ^ kernel/cgroup/rstat.c:503:19: note: (near initialization for ‘bpf_rstat_kfunc_set’) make[3]: *** [scripts/Makefile.build:288: kernel/cgroup/rstat.o] Error 1 Please fix. > +}; > + > +static int __init bpf_rstat_kfunc_init(void) > +{ > + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, > + &bpf_rstat_kfunc_set); > +} > +late_initcall(bpf_rstat_kfunc_init);
On Fri, May 20, 2022 at 4:16 AM Tejun Heo <tj@kernel.org> wrote: > > On Fri, May 20, 2022 at 03:06:07PM +0530, Kumar Kartikeya Dwivedi wrote: > > With static noinline, the compiler will optimize away the function. With global > > noinline, it can still optimize away the call site, but will keep the function > > definition, so attach works. Therefore __weak is needed to ensure call is still > > emitted. With GCC __attribute__((noipa)) might have been more appropritate, but > > LLVM doesn't support it, so __weak is the next best thing supported by both with > > the same side effect. Thanks a lot for the explanation! > > Ah, okay, so it's to prevent compiler from optimizing away call to a noop > function by telling it that we don't know what the function might eventually > be. Thanks for the explanation. Yosry, can you please add a comment > explaining what's going on? Will add a comment explaining things in the next section. Thanks for reviewing this, Tejun! > > Thanks. > > -- > tejun
On Fri, May 20, 2022 at 8:15 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 5/19/22 6:21 PM, Yosry Ahmed wrote: > > Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > > tracing programs. bpf programs that make use of rstat can use these > > functions to inform rstat when they update stats for a cgroup, and when > > they need to flush the stats. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > --- > > kernel/cgroup/rstat.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > > index e7a88d2600bd..a16a851bc0a1 100644 > > --- a/kernel/cgroup/rstat.c > > +++ b/kernel/cgroup/rstat.c > > @@ -3,6 +3,11 @@ > > > > #include <linux/sched/cputime.h> > > > > +#include <linux/bpf.h> > > +#include <linux/btf.h> > > +#include <linux/btf_ids.h> > > + > > + > > static DEFINE_SPINLOCK(cgroup_rstat_lock); > > static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); > > > > @@ -141,7 +146,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos, > > return pos; > > } > > > > -/* A hook for bpf stat collectors to attach to and flush their stats */ > > +/* > > + * A hook for bpf stat collectors to attach to and flush their stats. > > + * Together with providing bpf kfuncs for cgroup_rstat_updated() and > > + * cgroup_rstat_flush(), this enables a complete workflow where bpf progs that > > + * collect cgroup stats can integrate with rstat for efficient flushing. > > + */ > > __weak noinline void bpf_rstat_flush(struct cgroup *cgrp, > > struct cgroup *parent, int cpu) > > { > > @@ -476,3 +486,26 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq) > > "system_usec %llu\n", > > usage, utime, stime); > > } > > + > > +/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */ > > +BTF_SET_START(bpf_rstat_check_kfunc_ids) > > +BTF_ID(func, cgroup_rstat_updated) > > +BTF_ID(func, cgroup_rstat_flush) > > +BTF_SET_END(bpf_rstat_check_kfunc_ids) > > + > > +BTF_SET_START(bpf_rstat_sleepable_kfunc_ids) > > +BTF_ID(func, cgroup_rstat_flush) > > +BTF_SET_END(bpf_rstat_sleepable_kfunc_ids) > > + > > +static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = { > > + .owner = THIS_MODULE, > > + .check_set = &bpf_rstat_check_kfunc_ids, > > + .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > > There is a compilation error here: > > kernel/cgroup/rstat.c:503:3: error: ‘const struct btf_kfunc_id_set’ has > no member named ‘sleepable_set’; did you mean ‘release_set’? > 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > | ^~~~~~~~~~~~~ > | release_set > kernel/cgroup/rstat.c:503:19: warning: excess elements in struct > initializer > 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > | ^ > kernel/cgroup/rstat.c:503:19: note: (near initialization for > ‘bpf_rstat_kfunc_set’) > make[3]: *** [scripts/Makefile.build:288: kernel/cgroup/rstat.o] Error 1 > > Please fix. This patch series is rebased on top of 2 patches in the mailing list: - bpf/btf: also allow kfunc in tracing and syscall programs - btf: Add a new kfunc set which allows to mark a function to be sleepable I specified this in the cover letter, do I need to do something else in this situation? Re-send the patches as part of my series? > > > +}; > > + > > +static int __init bpf_rstat_kfunc_init(void) > > +{ > > + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, > > + &bpf_rstat_kfunc_set); > > +} > > +late_initcall(bpf_rstat_kfunc_init);
On 5/20/22 9:08 AM, Yosry Ahmed wrote: > On Fri, May 20, 2022 at 8:15 AM Yonghong Song <yhs@fb.com> wrote: >> >> >> >> On 5/19/22 6:21 PM, Yosry Ahmed wrote: >>> Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf >>> tracing programs. bpf programs that make use of rstat can use these >>> functions to inform rstat when they update stats for a cgroup, and when >>> they need to flush the stats. >>> >>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com> >>> --- >>> kernel/cgroup/rstat.c | 35 ++++++++++++++++++++++++++++++++++- >>> 1 file changed, 34 insertions(+), 1 deletion(-) >>> >>> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c >>> index e7a88d2600bd..a16a851bc0a1 100644 >>> --- a/kernel/cgroup/rstat.c >>> +++ b/kernel/cgroup/rstat.c >>> @@ -3,6 +3,11 @@ >>> >>> #include <linux/sched/cputime.h> >>> >>> +#include <linux/bpf.h> >>> +#include <linux/btf.h> >>> +#include <linux/btf_ids.h> >>> + >>> + >>> static DEFINE_SPINLOCK(cgroup_rstat_lock); >>> static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); >>> >>> @@ -141,7 +146,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos, >>> return pos; >>> } >>> >>> -/* A hook for bpf stat collectors to attach to and flush their stats */ >>> +/* >>> + * A hook for bpf stat collectors to attach to and flush their stats. >>> + * Together with providing bpf kfuncs for cgroup_rstat_updated() and >>> + * cgroup_rstat_flush(), this enables a complete workflow where bpf progs that >>> + * collect cgroup stats can integrate with rstat for efficient flushing. >>> + */ >>> __weak noinline void bpf_rstat_flush(struct cgroup *cgrp, >>> struct cgroup *parent, int cpu) >>> { >>> @@ -476,3 +486,26 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq) >>> "system_usec %llu\n", >>> usage, utime, stime); >>> } >>> + >>> +/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */ >>> +BTF_SET_START(bpf_rstat_check_kfunc_ids) >>> +BTF_ID(func, cgroup_rstat_updated) >>> +BTF_ID(func, cgroup_rstat_flush) >>> +BTF_SET_END(bpf_rstat_check_kfunc_ids) >>> + >>> +BTF_SET_START(bpf_rstat_sleepable_kfunc_ids) >>> +BTF_ID(func, cgroup_rstat_flush) >>> +BTF_SET_END(bpf_rstat_sleepable_kfunc_ids) >>> + >>> +static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = { >>> + .owner = THIS_MODULE, >>> + .check_set = &bpf_rstat_check_kfunc_ids, >>> + .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, >> >> There is a compilation error here: >> >> kernel/cgroup/rstat.c:503:3: error: ‘const struct btf_kfunc_id_set’ has >> no member named ‘sleepable_set’; did you mean ‘release_set’? >> 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, >> | ^~~~~~~~~~~~~ >> | release_set >> kernel/cgroup/rstat.c:503:19: warning: excess elements in struct >> initializer >> 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, >> | ^ >> kernel/cgroup/rstat.c:503:19: note: (near initialization for >> ‘bpf_rstat_kfunc_set’) >> make[3]: *** [scripts/Makefile.build:288: kernel/cgroup/rstat.o] Error 1 >> >> Please fix. > > This patch series is rebased on top of 2 patches in the mailing list: > - bpf/btf: also allow kfunc in tracing and syscall programs > - btf: Add a new kfunc set which allows to mark a function to be > sleepable > > I specified this in the cover letter, do I need to do something else > in this situation? Re-send the patches as part of my series? At least put a link in the cover letter for the above two patches? This way, people can easily find them to double check. > > > >> >>> +}; >>> + >>> +static int __init bpf_rstat_kfunc_init(void) >>> +{ >>> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, >>> + &bpf_rstat_kfunc_set); >>> +} >>> +late_initcall(bpf_rstat_kfunc_init);
On Fri, May 20, 2022 at 9:16 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 5/20/22 9:08 AM, Yosry Ahmed wrote: > > On Fri, May 20, 2022 at 8:15 AM Yonghong Song <yhs@fb.com> wrote: > >> > >> > >> > >> On 5/19/22 6:21 PM, Yosry Ahmed wrote: > >>> Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf > >>> tracing programs. bpf programs that make use of rstat can use these > >>> functions to inform rstat when they update stats for a cgroup, and when > >>> they need to flush the stats. > >>> > >>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > >>> --- > >>> kernel/cgroup/rstat.c | 35 ++++++++++++++++++++++++++++++++++- > >>> 1 file changed, 34 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > >>> index e7a88d2600bd..a16a851bc0a1 100644 > >>> --- a/kernel/cgroup/rstat.c > >>> +++ b/kernel/cgroup/rstat.c > >>> @@ -3,6 +3,11 @@ > >>> > >>> #include <linux/sched/cputime.h> > >>> > >>> +#include <linux/bpf.h> > >>> +#include <linux/btf.h> > >>> +#include <linux/btf_ids.h> > >>> + > >>> + > >>> static DEFINE_SPINLOCK(cgroup_rstat_lock); > >>> static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); > >>> > >>> @@ -141,7 +146,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos, > >>> return pos; > >>> } > >>> > >>> -/* A hook for bpf stat collectors to attach to and flush their stats */ > >>> +/* > >>> + * A hook for bpf stat collectors to attach to and flush their stats. > >>> + * Together with providing bpf kfuncs for cgroup_rstat_updated() and > >>> + * cgroup_rstat_flush(), this enables a complete workflow where bpf progs that > >>> + * collect cgroup stats can integrate with rstat for efficient flushing. > >>> + */ > >>> __weak noinline void bpf_rstat_flush(struct cgroup *cgrp, > >>> struct cgroup *parent, int cpu) > >>> { > >>> @@ -476,3 +486,26 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq) > >>> "system_usec %llu\n", > >>> usage, utime, stime); > >>> } > >>> + > >>> +/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */ > >>> +BTF_SET_START(bpf_rstat_check_kfunc_ids) > >>> +BTF_ID(func, cgroup_rstat_updated) > >>> +BTF_ID(func, cgroup_rstat_flush) > >>> +BTF_SET_END(bpf_rstat_check_kfunc_ids) > >>> + > >>> +BTF_SET_START(bpf_rstat_sleepable_kfunc_ids) > >>> +BTF_ID(func, cgroup_rstat_flush) > >>> +BTF_SET_END(bpf_rstat_sleepable_kfunc_ids) > >>> + > >>> +static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = { > >>> + .owner = THIS_MODULE, > >>> + .check_set = &bpf_rstat_check_kfunc_ids, > >>> + .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > >> > >> There is a compilation error here: > >> > >> kernel/cgroup/rstat.c:503:3: error: ‘const struct btf_kfunc_id_set’ has > >> no member named ‘sleepable_set’; did you mean ‘release_set’? > >> 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > >> | ^~~~~~~~~~~~~ > >> | release_set > >> kernel/cgroup/rstat.c:503:19: warning: excess elements in struct > >> initializer > >> 503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, > >> | ^ > >> kernel/cgroup/rstat.c:503:19: note: (near initialization for > >> ‘bpf_rstat_kfunc_set’) > >> make[3]: *** [scripts/Makefile.build:288: kernel/cgroup/rstat.o] Error 1 > >> > >> Please fix. > > > > This patch series is rebased on top of 2 patches in the mailing list: > > - bpf/btf: also allow kfunc in tracing and syscall programs > > - btf: Add a new kfunc set which allows to mark a function to be > > sleepable > > > > I specified this in the cover letter, do I need to do something else > > in this situation? Re-send the patches as part of my series? > > At least put a link in the cover letter for the above two patches? > This way, people can easily find them to double check. Right. Will do this in the next version. Sorry for the inconvenience. > > > > > > > > >> > >>> +}; > >>> + > >>> +static int __init bpf_rstat_kfunc_init(void) > >>> +{ > >>> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, > >>> + &bpf_rstat_kfunc_set); > >>> +} > >>> +late_initcall(bpf_rstat_kfunc_init);
Hi Yosry,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Yosry-Ahmed/bpf-rstat-cgroup-hierarchical-stats/20220520-093041
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20220521/202205211913.wPnVDaPm-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/203797424b1159b12702cea9d9a20acc24ea92e0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yosry-Ahmed/bpf-rstat-cgroup-hierarchical-stats/20220520-093041
git checkout 203797424b1159b12702cea9d9a20acc24ea92e0
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash kernel/cgroup/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
kernel/cgroup/rstat.c:155:22: warning: no previous prototype for 'bpf_rstat_flush' [-Wmissing-prototypes]
155 | __weak noinline void bpf_rstat_flush(struct cgroup *cgrp,
| ^~~~~~~~~~~~~~~
kernel/cgroup/rstat.c:503:10: error: 'const struct btf_kfunc_id_set' has no member named 'sleepable_set'; did you mean 'release_set'?
503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids,
| ^~~~~~~~~~~~~
| release_set
>> kernel/cgroup/rstat.c:503:27: warning: excess elements in struct initializer
503 | .sleepable_set = &bpf_rstat_sleepable_kfunc_ids,
| ^
kernel/cgroup/rstat.c:503:27: note: (near initialization for 'bpf_rstat_kfunc_set')
vim +503 kernel/cgroup/rstat.c
499
500 static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = {
501 .owner = THIS_MODULE,
502 .check_set = &bpf_rstat_check_kfunc_ids,
> 503 .sleepable_set = &bpf_rstat_sleepable_kfunc_ids,
504 };
505
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c index e7a88d2600bd..a16a851bc0a1 100644 --- a/kernel/cgroup/rstat.c +++ b/kernel/cgroup/rstat.c @@ -3,6 +3,11 @@ #include <linux/sched/cputime.h> +#include <linux/bpf.h> +#include <linux/btf.h> +#include <linux/btf_ids.h> + + static DEFINE_SPINLOCK(cgroup_rstat_lock); static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); @@ -141,7 +146,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos, return pos; } -/* A hook for bpf stat collectors to attach to and flush their stats */ +/* + * A hook for bpf stat collectors to attach to and flush their stats. + * Together with providing bpf kfuncs for cgroup_rstat_updated() and + * cgroup_rstat_flush(), this enables a complete workflow where bpf progs that + * collect cgroup stats can integrate with rstat for efficient flushing. + */ __weak noinline void bpf_rstat_flush(struct cgroup *cgrp, struct cgroup *parent, int cpu) { @@ -476,3 +486,26 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq) "system_usec %llu\n", usage, utime, stime); } + +/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */ +BTF_SET_START(bpf_rstat_check_kfunc_ids) +BTF_ID(func, cgroup_rstat_updated) +BTF_ID(func, cgroup_rstat_flush) +BTF_SET_END(bpf_rstat_check_kfunc_ids) + +BTF_SET_START(bpf_rstat_sleepable_kfunc_ids) +BTF_ID(func, cgroup_rstat_flush) +BTF_SET_END(bpf_rstat_sleepable_kfunc_ids) + +static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = { + .owner = THIS_MODULE, + .check_set = &bpf_rstat_check_kfunc_ids, + .sleepable_set = &bpf_rstat_sleepable_kfunc_ids, +}; + +static int __init bpf_rstat_kfunc_init(void) +{ + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, + &bpf_rstat_kfunc_set); +} +late_initcall(bpf_rstat_kfunc_init);
Add cgroup_rstat_updated() and cgroup_rstat_flush() kfuncs to bpf tracing programs. bpf programs that make use of rstat can use these functions to inform rstat when they update stats for a cgroup, and when they need to flush the stats. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- kernel/cgroup/rstat.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)