cgroup, sched: add bpf for cgroup cpu controller

[kernel-patches-daemon-bpf]

Pull request for series with
subject: cgroup, sched: add bpf for cgroup cpu controller
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678

[kernel-patches-daemon-bpf]

Upstream branch: 806c1a1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-review-bot]

commit 961f08b5b3608ccdb219f4f83a13a890f3a77a67
Author: Ziyang Men <ziyang.meme@gmail.com>

selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs

Add cgroup_iter_cpu, a selftest for the added CPU controller BPF kfuncs.

> The userspace side runs a CPU hog in a test cgroup with cpu.max settled
> then:
>   - checks the CPU-time and throttling counters are nonzero,
>   - compares whether all values the program read are same as those
>     reading from cgroup file.

This isn't a bug, but could these two sentences be reworded? The phrase
"with cpu.max settled" is a bit unclear (the test writes a quota to
cpu.max), and "compares whether all values the program read are same as
those reading from cgroup file" has an article/verb agreement issue.
Perhaps "with a quota set in cpu.max" and "compares the values the program
read against those read from the cgroup files"?

Also, the test has to wait for cpu.stat to stop moving before it can
compare the two sources - would it be worth mentioning that retry loop
since it's the least obvious part of the test?

> CONFIG_CGROUP_SCHED, CONFIG_FAIR_GROUP_SCHED and CONFIG_CFS_BANDWIDTH
> are added to the test config.
>
> Tests passed on v7.2-rc5.
>
> Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>

> diff --git a/tools/testing/selftests/bpf/cgroup_iter_cpu.h b/tools/testing/selftests/bpf/cgroup_iter_cpu.h
> new file mode 100644
> index 0000000000000..74599a5c0e4d9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/cgroup_iter_cpu.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
> +#ifndef __CGROUP_ITER_CPU_H

This isn't a bug, but the copyright line here matches cgroup_iter_memcg.h
exactly - is the Meta Platforms attribution (and the 2025 year) the
intended one for these new files, or did it come along with the template?
The same line appears in all three new files (cgroup_iter_cpu.h,
prog_tests/cgroup_iter_cpu.c, progs/cgroup_iter_cpu.c), but the assigned
entity doesn't match the Signed-off-by address, and the year predates the
commit date (Aug 2026).

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c
> new file mode 100644

[ ... ]

> +static int read_stats(struct bpf_link *link)
> +{
> +	int fd, ret = 0;
> +	ssize_t bytes;
> +
> +	fd = bpf_iter_create(bpf_link__fd(link));
> +	if (!ASSERT_OK_FD(fd, "bpf_iter_create"))
> +		return 1;
> +
> +	bytes = read(fd, NULL, 0);
> +	if (!ASSERT_EQ(bytes, 0, "read fd"))
> +		ret = 1;
> +
> +	close(fd);
> +	return ret;
> +}

This isn't a bug, but read_stats() looks identical to the one in
cgroup_iter_memcg.c - would it be worth sharing it (and the comment
explaining the read(fd, NULL, 0) idiom) so the next .data.query-style
iter test can reuse it? The memcg version documents why read(fd, NULL, 0)
is the right way to run the iterator, whereas here a reader is left to
work out why reading zero bytes triggers the program.

[ ... ]

> +/* Parse the "cpu.stat" file into @out. */
> +static int parse_cpu_stat(int cgroup_fd, struct cpu_query *out)

[ ... ]

> +/*
> + * Parse the "cpu.stat.local" file into @out.
> + */
> +static int parse_cpu_stat_local(int cgroup_fd, struct cpu_query *out)

[ ... ]

> +/* Read file value the bpf program reads. */
> +static int parse_stats(int cgroup_fd, struct cpu_query *out, bool have_bw)

[ ... ]

> +/*
> + * Check whether this kernel accounts CFS bandwidth.
> + */
> +static bool cgroup_has_bw_stat(int cgroup_fd)

This isn't a bug, but would it read more consistently to use the
single-line comment form for the one-line banners on
parse_cpu_stat_local()/cgroup_has_bw_stat()? The file uses two different
forms for equally short one-sentence comments: single-line /* ... */ for
read_cgroup_file, parse_cpu_stat and parse_stats, but a three-line block
for parse_cpu_stat_local and cgroup_has_bw_stat.

Also, the parse_stats() comment "Read file value the bpf program reads" is
hard to parse and says nothing about the have_bw parameter, which is the
only thing about that function that isn't obvious from its body. And
"Check whether this kernel accounts CFS bandwidth" describes
cgroup_has_bw_stat() slightly more broadly than it acts, since reading
'nr_periods ' out of cpu.stat also depends on the cpu controller being
enabled for the cgroup, not only on the kernel config.

[ ... ]

> +	if (test__start_subtest("cgroup_iter_cpu__throttling")) {
> +		if (!have_bw) {
> +			test__skip();

A subsystem pattern flags this as potentially concerning: the
cgroup_iter_cpu__throttling subtest skips with a bare test__skip() and
prints nothing, so the log records a skip with no indication of which
prerequisite was absent. have_bw is the conjunction of two quite different
conditions -- wrote_max (the write to cpu.max succeeded) and
cgroup_has_bw_stat() (cpu.stat contains 'nr_periods ', i.e. the kernel was
built with CONFIG_CFS_BANDWIDTH) -- and the skip collapses both into no
output.

Could a printf("%s:SKIP:...", __func__) naming the missing prerequisite be
added before test__skip()? That would make the skip self-describing for
someone reading a CI log who can't tell whether the kernel lacks
CONFIG_CFS_BANDWIDTH, the cpu controller was not delegated, or the cpu.max
write failed for some other reason.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/cgroup_iter_cpu.c b/tools/testing/selftests/bpf/progs/cgroup_iter_cpu.c
> new file mode 100644

[ ... ]

> +	bpf_rcu_read_lock();
> +	css = cgrp->subsys[cpu_cgrp_id];
> +	tg = (struct task_group *)css;
> +	if (tg && bpf_core_field_exists(tg->cfs_bandwidth.nr_periods)) {
> +		cpu_query.nr_periods =
> +			(__u32)BPF_CORE_READ(tg, cfs_bandwidth.nr_periods);
> +		cpu_query.nr_throttled =
> +			(__u32)BPF_CORE_READ(tg, cfs_bandwidth.nr_throttled);
> +		cpu_query.throttled_usec =
> +			BPF_CORE_READ(tg, cfs_bandwidth.throttled_time) / 1000;
> +		cpu_query.nr_bursts =
> +			(__u32)BPF_CORE_READ(tg, cfs_bandwidth.nr_burst);
> +		cpu_query.burst_usec =
> +			BPF_CORE_READ(tg, cfs_bandwidth.burst_time) / 1000;
> +	}
> +	bpf_rcu_read_unlock();

This isn't a bug, but would it read more consistently to fetch the
cpu_cgrp_id index with bpf_core_enum_value(enum cgroup_subsys_id,
cpu_cgrp_id), the way cgroup_iter_memcg.c and profiler.inc.h do? The
struct field chain right below is accessed CO-RE style (with
bpf_core_field_exists and BPF_CORE_READ), but the array index cpu_cgrp_id
is compiled in as a plain enum constant from vmlinux.h with no relocation.
enum cgroup_subsys_id's numbering shifts with the set of enabled
controllers, so the two halves of this access are hardened differently.

---

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31735302699

[kernel-patches-review-bot]

commit 2bded39d4de9a3b5b9cf1562017aaddea7b5c269
Author: Ziyang Men <ziyang.meme@gmail.com>

cgroup, sched: add BPF kfuncs to read a cpu cgroup's stats

This commit adds BPF kfuncs for the cgroup CPU controller, following the
memory controller kfuncs in mm/bpf_memcontrol.c. The design leaves CFS
bandwidth counter reads to BPF programs and adds kfuncs for throttled
time computation and base CPU-time values.

> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -923,4 +923,19 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
>
>  struct cgroup_of_peak *of_peak(struct kernfs_open_file *of);
>
> +/* A cgroup's base CPU-time counters in microseconds, as cpu.stat prints them */
> +struct cpu_cgroup_cputime {
> +	u64 usage_usec;
> +	u64 user_usec;
> +	u64 system_usec;
> +	u64 nice_usec;
> +	u64 forceidle_usec;	/* 0 without CONFIG_SCHED_CORE */
> +};
> +
> +/* A task_group's own throttled time in nanoseconds; see cpu.stat.local */
> +struct task_group;
> +#ifdef CONFIG_CFS_BANDWIDTH
> +u64 throttled_time_self(struct task_group *tg);
> +#endif
> +
>  #endif /* _LINUX_CGROUP_H */

Should this scheduler interface be in include/linux/cgroup.h? The header
now forward-declares struct task_group and declares a CFS-bandwidth
function, although task_group and its accessors live in
kernel/sched/sched.h.

Nothing else in include/linux/cgroup.h references scheduler-internal
types. The only consumer is kernel/cgroup/bpf_cpu.c, which could get it
from kernel/cgroup/cgroup-internal.h instead (where the patch already
puts bpf_cpu_cgroup_cputime()).

Also, throttled_time_self() becomes a global, unprefixed symbol when it
was previously file-local to kernel/sched/core.c. Would a name like
sched_cgroup_throttled_time_self() avoid polluting the global namespace,
or could a small prefixed wrapper keep the original function static?

> diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
> --- a/kernel/cgroup/Makefile
> +++ b/kernel/cgroup/Makefile

[ ... ]

> diff --git a/kernel/cgroup/bpf_cpu.c b/kernel/cgroup/bpf_cpu.c
> --- /dev/null
> +++ b/kernel/cgroup/bpf_cpu.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CPU Controller-related BPF kfuncs
> + *
> + * bpf_cpu_cgroup_cputime() is defined in rstat.c, which owns the locking it
> + * needs, and only registered here.
> + *
> + * Author: Ziyang Men <ziyang.meme@gmail.com>
> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/cgroup.h>
> +
> +#include "cgroup-internal.h"
> +
> +__bpf_kfunc_start_defs();

[ ... ]

> +/**
> + * bpf_cpu_cgroup_throttled_self - Read a cgroup's own throttled time
> + * @cgrp: cgroup to read from
> + *
> + * Return: The throttled time in microseconds, or 0 if config is off.
> + */
> +__bpf_kfunc u64 bpf_cpu_cgroup_throttled_self(struct cgroup *cgrp)
> +{
> +/* cpu_cgrp_id needs the cpu controller, which CFS bandwidth depends on */
> +#ifdef CONFIG_CFS_BANDWIDTH
> +	struct cgroup_subsys_state *css;
> +
> +	guard(rcu)();
> +
> +	css = rcu_dereference(cgrp->subsys[cpu_cgrp_id]);
> +	if (!css)
> +		return 0;
> +
> +	return div_u64(throttled_time_self((struct task_group *)css),
> +		       NSEC_PER_USEC);

Is the raw cast to struct task_group * safe here? This works because
struct cgroup_subsys_state css is the first member of struct task_group,
but struct task_group is an incomplete type in this translation unit.

kernel/cgroup/bpf_cpu.c includes only <linux/bpf.h>, <linux/btf.h>,
<linux/cgroup.h> and "cgroup-internal.h", so the compiler cannot verify
the cast.

Every existing css->task_group conversion in the tree uses css_tg() from
kernel/sched/sched.h, which is container_of() and stays correct if the
member ever moves. The scheduler helpers cpu_cfs_local_stat_show() and
cpu_local_stat_show() do the same conversion with css_tg().

Would moving the cpu-controller kfunc to kernel/sched/core.c (where
struct task_group is complete and css_tg() is available) avoid hard-
coding a struct-layout invariant that this file cannot check?

> +#else
> +	return 0;
> +#endif
> +}

[ ... ]

> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c

[ ... ]

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10027,7 +10027,7 @@ static int cpu_cfs_stat_show(struct seq_file *sf, void *v)
>  	return 0;
>  }
>
> -static u64 throttled_time_self(struct task_group *tg)
> +u64 throttled_time_self(struct task_group *tg)
>  {
>  	int i;
>  	u64 total = 0;

---

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: cgroup, sched: add BPF kfuncs to read a cpu cgroup's stats
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31735302699

[kernel-patches-daemon-bpf]

Forwarding comment 5285813709 via email
In-Reply-To: 20260813185846.1216892-3-ziyang.meme@gmail.com
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260813185846.1216892-3-ziyang.meme@gmail.com/

[kernel-patches-daemon-bpf]

Forwarding comment 5285824105 via email
In-Reply-To: 20260813185846.1216892-2-ziyang.meme@gmail.com
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260813185846.1216892-2-ziyang.meme@gmail.com/

[kernel-patches-daemon-bpf]

Upstream branch: aacd13e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 259d60f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 4d9551b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 409a9bd
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: c7e6175
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: f2aaa62
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 8eb1892
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: d99bda7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 34e0eb7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 5fe7007
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 77877bf
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: adb7719
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 6b0835a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 6ab6a94
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1

[kernel-patches-daemon-bpf]

Upstream branch: 3d9393f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1145678
version: 1