[PATCH 0/2] cgroup, sched: add bpf for cgroup cpu controller

[PATCH 0/2] cgroup, sched: add bpf for cgroup cpu controller

[Ziyang Men]

As the title suggests, this series adds bpf kfuncs for the cgroup CPU
controller.

Motivation:
Collecting cgroup statistics is expensive: the existing
method is to open and parse a cgroup file. memcg already has an
efficient alternative through BPF; this series extends that idea to cpu.

Design:
- Leave reading the CFS bandwidth counters to the BPF program. They are
  plain fields of tg->cfs_bandwidth, so they need no kernel code.
- Add one kfunc to compute the throttled time. This is necessary because
  it is a sum over every possible cpu, which a user cannot do itself.

The only part it touches the scheduler part is to discard the static for
throttled_time_self() in order to use externally.

Patch 1 adds the kfuncs (kernel/cgroup/bpf_cpu.c); patch 2 adds a
test_progs selftest.

Tested on VM with v7.2-rc5. 

Ziyang Men (2):
  cgroup, sched: add BPF kfuncs to read a cpu cgroup's stats
  selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs

 include/linux/cgroup.h                        |  15 +
 kernel/cgroup/Makefile                        |   2 +
 kernel/cgroup/bpf_cpu.c                       |  80 ++++++
 kernel/cgroup/cgroup-internal.h               |   3 +
 kernel/cgroup/rstat.c                         |  42 +++
 kernel/sched/core.c                           |   2 +-
 tools/testing/selftests/bpf/cgroup_iter_cpu.h |  22 ++
 tools/testing/selftests/bpf/config            |   3 +
 .../bpf/prog_tests/cgroup_iter_cpu.c          | 259 ++++++++++++++++++
 .../selftests/bpf/progs/cgroup_iter_cpu.c     |  53 ++++
 10 files changed, 480 insertions(+), 1 deletion(-)
 create mode 100644 kernel/cgroup/bpf_cpu.c
 create mode 100644 tools/testing/selftests/bpf/cgroup_iter_cpu.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c
 create mode 100644 tools/testing/selftests/bpf/progs/cgroup_iter_cpu.c

-- 
2.53.0-Meta

[PATCH 1/2] cgroup, sched: add BPF kfuncs to read a cpu cgroup's stats

[Ziyang Men]

This series adds bpf kfuncs for the cgroup CPU controller, following the
memory controller kfuncs in mm/bpf_memcontrol.c.

Collecting cgroup statistics is expensive: the existing
method is to open and parse a cgroup file. memcg already has an
efficient alternative through BPF; this series extends that idea to cpu.

Design:
- Leave reading the CFS bandwidth counters to the BPF program. They are
  plain fields of tg->cfs_bandwidth, so they need no kernel code.
- Add one kfunc to compute the throttled time. This is necessary because
  it is a sum over every possible cpu, which a user cannot do itself.
- The bpf_cpu_cgroup_cputime() returns all five base CPU-time values in one
  call with one cputime_adjust().

The only part it touches the scheduler part is to discard the static for
throttled_time_self() in order to use externally.

The two kfuncs that take the rstat lock are KF_SLEEPABLE following idea
in the mm/bpf_memcontrol.c

Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 include/linux/cgroup.h          | 15 +++++++
 kernel/cgroup/Makefile          |  2 +
 kernel/cgroup/bpf_cpu.c         | 80 +++++++++++++++++++++++++++++++++
 kernel/cgroup/cgroup-internal.h |  3 ++
 kernel/cgroup/rstat.c           | 42 +++++++++++++++++
 kernel/sched/core.c             |  2 +-
 6 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 kernel/cgroup/bpf_cpu.c

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index f2aa46a4f871..d2a6b5efad51 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -923,4 +923,19 @@ struct cgroup *task_get_cgroup1(struct task_struct *tsk, int hierarchy_id);
 
 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 */
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index ede31601a363..0ba59b7eef48 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o freezer.o
 
+obj-$(CONFIG_BPF_SYSCALL) += bpf_cpu.o
+
 obj-$(CONFIG_CGROUP_FREEZER) += legacy_freezer.o
 obj-$(CONFIG_CGROUP_PIDS) += pids.o
 obj-$(CONFIG_CGROUP_RDMA) += rdma.o
diff --git a/kernel/cgroup/bpf_cpu.c b/kernel/cgroup/bpf_cpu.c
new file mode 100644
index 000000000000..6eb89c8e84fd
--- /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_flush_stats - Flush a cgroup's base CPU-time statistics
+ * @cgrp: cgroup to flush
+ *
+ * Propagate the cgroup's base CPU-time statistics up the cgroup tree.
+ */
+__bpf_kfunc void bpf_cpu_cgroup_flush_stats(struct cgroup *cgrp)
+{
+	css_rstat_flush(&cgrp->self);
+}
+
+/**
+ * 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);
+#else
+	return 0;
+#endif
+}
+
+__bpf_kfunc_end_defs();
+
+/* KF_SLEEPABLE keeps the rstat spinlock out of NMI */
+BTF_KFUNCS_START(bpf_cpu_cgroup_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_cpu_cgroup_flush_stats, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_cpu_cgroup_cputime, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_cpu_cgroup_throttled_self)
+BTF_KFUNCS_END(bpf_cpu_cgroup_kfunc_ids)
+
+static const struct btf_kfunc_id_set bpf_cpu_cgroup_kfunc_set = {
+	.owner		= THIS_MODULE,
+	.set		= &bpf_cpu_cgroup_kfunc_ids,
+};
+
+static int __init bpf_cpu_cgroup_kfunc_init(void)
+{
+	int err;
+
+	err = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
+					&bpf_cpu_cgroup_kfunc_set);
+	if (err)
+		pr_warn("error while registering cpu cgroup kfuncs: %d\n", err);
+
+	return err;
+}
+late_initcall(bpf_cpu_cgroup_kfunc_init);
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 58797123b752..65f5b6318289 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -271,6 +271,9 @@ int css_rstat_init(struct cgroup_subsys_state *css);
 void css_rstat_exit(struct cgroup_subsys_state *css);
 int ss_rstat_init(struct cgroup_subsys *ss);
 void cgroup_base_stat_cputime_show(struct seq_file *seq);
+#ifdef CONFIG_BPF_SYSCALL
+void bpf_cpu_cgroup_cputime(struct cgroup *cgrp, struct cpu_cgroup_cputime *out);
+#endif
 
 /*
  * namespace.c
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index de816a43db9f..f9e30719068e 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -752,6 +752,48 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq)
 	cgroup_force_idle_show(seq, &bstat);
 }
 
+#ifdef CONFIG_BPF_SYSCALL
+
+__bpf_kfunc_start_defs();
+
+/**
+ * bpf_cpu_cgroup_cputime - Read a cgroup's base CPU-time data
+ * @cgrp: cgroup to read from
+ * @out: the data in microseconds. Zero it first: the verifier reads the
+ *       whole struct.
+ *
+ * Adjust once and fill all values.
+ */
+__bpf_kfunc void bpf_cpu_cgroup_cputime(struct cgroup *cgrp,
+					struct cpu_cgroup_cputime *out)
+{
+	struct cgroup_base_stat bstat;
+
+	if (cgroup_parent(cgrp)) {
+		__css_rstat_lock(&cgrp->self, -1);
+		bstat = cgrp->bstat;
+		cputime_adjust(&cgrp->bstat.cputime, &cgrp->prev_cputime,
+			       &bstat.cputime.utime, &bstat.cputime.stime);
+		__css_rstat_unlock(&cgrp->self, -1);
+	} else {
+		root_cgroup_cputime(&bstat);
+	}
+
+	out->usage_usec = div_u64(bstat.cputime.sum_exec_runtime, NSEC_PER_USEC);
+	out->user_usec = div_u64(bstat.cputime.utime, NSEC_PER_USEC);
+	out->system_usec = div_u64(bstat.cputime.stime, NSEC_PER_USEC);
+	out->nice_usec = div_u64(bstat.ntime, NSEC_PER_USEC);
+#ifdef CONFIG_SCHED_CORE
+	out->forceidle_usec = div_u64(bstat.forceidle_sum, NSEC_PER_USEC);
+#else
+	out->forceidle_usec = 0;
+#endif
+}
+
+__bpf_kfunc_end_defs();
+
+#endif /* CONFIG_BPF_SYSCALL */
+
 /* Add bpf kfuncs for css_rstat_updated() and css_rstat_flush() */
 BTF_KFUNCS_START(bpf_rstat_kfunc_ids)
 BTF_ID_FLAGS(func, css_rstat_updated)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 96226707c2f6..75735e0e81ef 100644
--- 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;
-- 
2.53.0-Meta

[PATCH 2/2] selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs

[Ziyang Men]

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.

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>
---
 tools/testing/selftests/bpf/cgroup_iter_cpu.h |  22 ++
 tools/testing/selftests/bpf/config            |   3 +
 .../bpf/prog_tests/cgroup_iter_cpu.c          | 259 ++++++++++++++++++
 .../selftests/bpf/progs/cgroup_iter_cpu.c     |  53 ++++
 4 files changed, 337 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/cgroup_iter_cpu.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c
 create mode 100644 tools/testing/selftests/bpf/progs/cgroup_iter_cpu.c

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 000000000000..74599a5c0e4d
--- /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
+#define __CGROUP_ITER_CPU_H
+
+struct cpu_query {
+	/* base cpu time, from cpu.stat */
+	__u64 usage_usec;
+	__u64 user_usec;
+	__u64 system_usec;
+	__u64 nice_usec;
+	__u64 forceidle_usec;
+	/* CFS bandwidth throttling, from cpu.stat and cpu.stat.local */
+	__u64 nr_periods;
+	__u64 nr_throttled;
+	__u64 throttled_usec;
+	__u64 nr_bursts;
+	__u64 burst_usec;
+	__u64 throttled_self_usec;
+};
+
+#endif /* __CGROUP_ITER_CPU_H */
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index ea7044f30adc..482b40dde2f9 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -11,6 +11,9 @@ CONFIG_BPF_STREAM_PARSER=y
 CONFIG_BPF_SYSCALL=y
 # CONFIG_BPF_UNPRIV_DEFAULT_OFF is not set
 CONFIG_CGROUP_BPF=y
+CONFIG_CGROUP_SCHED=y
+CONFIG_FAIR_GROUP_SCHED=y
+CONFIG_CFS_BANDWIDTH=y
 CONFIG_CRYPTO_HMAC=y
 CONFIG_CRYPTO_SHA256=y
 CONFIG_CRYPTO_USER_API=y
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
index 000000000000..cd7e92ababfb
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_cpu.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <bpf/libbpf.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include "cgroup_helpers.h"
+#include "cgroup_iter_cpu.h"
+#include "cgroup_iter_cpu.skel.h"
+
+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;
+}
+
+/* Read cgroup file @name into @buf. */
+static int read_cgroup_file(int cgroup_fd, const char *name, char *buf,
+			    size_t size)
+{
+	ssize_t n;
+	int fd;
+
+	fd = openat(cgroup_fd, name, O_RDONLY);
+	if (fd < 0)
+		return -1;
+	n = read(fd, buf, size - 1);
+	close(fd);
+	if (n <= 0)
+		return -1;
+	buf[n] = '\0';
+	return 0;
+}
+
+/* Parse the "cpu.stat" file into @out. */
+static int parse_cpu_stat(int cgroup_fd, struct cpu_query *out)
+{
+	char buf[4096], *line, *sp;
+	unsigned long long v;
+
+	if (read_cgroup_file(cgroup_fd, "cpu.stat", buf, sizeof(buf)))
+		return -1;
+
+	for (line = strtok_r(buf, "\n", &sp); line;
+	     line = strtok_r(NULL, "\n", &sp)) {
+		if (sscanf(line, "usage_usec %llu", &v) == 1)
+			out->usage_usec = v;
+		else if (sscanf(line, "user_usec %llu", &v) == 1)
+			out->user_usec = v;
+		else if (sscanf(line, "system_usec %llu", &v) == 1)
+			out->system_usec = v;
+		else if (sscanf(line, "nice_usec %llu", &v) == 1)
+			out->nice_usec = v;
+		else if (sscanf(line, "core_sched.force_idle_usec %llu", &v) == 1)
+			out->forceidle_usec = v;
+		else if (sscanf(line, "nr_periods %llu", &v) == 1)
+			out->nr_periods = v;
+		else if (sscanf(line, "nr_throttled %llu", &v) == 1)
+			out->nr_throttled = v;
+		else if (sscanf(line, "throttled_usec %llu", &v) == 1)
+			out->throttled_usec = v;
+		else if (sscanf(line, "nr_bursts %llu", &v) == 1)
+			out->nr_bursts = v;
+		else if (sscanf(line, "burst_usec %llu", &v) == 1)
+			out->burst_usec = v;
+	}
+	return 0;
+}
+
+/*
+ * Parse the "cpu.stat.local" file into @out.
+ */
+static int parse_cpu_stat_local(int cgroup_fd, struct cpu_query *out)
+{
+	unsigned long long v;
+	char buf[256];
+
+	if (read_cgroup_file(cgroup_fd, "cpu.stat.local", buf, sizeof(buf)))
+		return -1;
+	if (sscanf(buf, "throttled_usec %llu", &v) != 1)
+		return -1;
+	out->throttled_self_usec = v;
+	return 0;
+}
+
+/* Read file value the bpf program reads. */
+static int parse_stats(int cgroup_fd, struct cpu_query *out, bool have_bw)
+{
+	if (parse_cpu_stat(cgroup_fd, out))
+		return -1;
+	if (have_bw && parse_cpu_stat_local(cgroup_fd, out))
+		return -1;
+	return 0;
+}
+
+/*
+ * Check whether this kernel accounts CFS bandwidth.
+ */
+static bool cgroup_has_bw_stat(int cgroup_fd)
+{
+	char buf[4096];
+
+	if (read_cgroup_file(cgroup_fd, "cpu.stat", buf, sizeof(buf)))
+		return false;
+	return strstr(buf, "nr_periods ");
+}
+
+/* Fork a child that spins in the current cgroup, kill it if the test exits. */
+static pid_t spawn_cpu_hog(void)
+{
+	pid_t pid = fork();
+
+	if (pid == 0) {
+		prctl(PR_SET_PDEATHSIG, SIGKILL);
+		while (1)
+			;
+	}
+	return pid;
+}
+
+void test_cgroup_iter_cpu(void)
+{
+	char *cgroup_rel_path = "/cgroup_iter_cpu_test";
+	struct cgroup_iter_cpu *skel;
+	struct cpu_query *q;
+	struct bpf_link *link;
+	bool wrote_max, have_bw;
+	int cgroup_fd;
+	pid_t hog;
+
+	cgroup_fd = cgroup_setup_and_join(cgroup_rel_path);
+	if (!ASSERT_OK_FD(cgroup_fd, "cgroup_setup_and_join"))
+		return;
+
+	wrote_max = !write_cgroup_file(cgroup_rel_path, "cpu.max", "10000 100000");
+
+	skel = cgroup_iter_cpu__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "cgroup_iter_cpu__open_and_load"))
+		goto cleanup_cgroup_fd;
+
+	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
+	union bpf_iter_link_info linfo = {
+		.cgroup.cgroup_fd = cgroup_fd,
+		.cgroup.order = BPF_CGROUP_ITER_SELF_ONLY,
+	};
+	opts.link_info = &linfo;
+	opts.link_info_len = sizeof(linfo);
+
+	link = bpf_program__attach_iter(skel->progs.cgroup_cpu_query, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_iter"))
+		goto cleanup_skel;
+
+	q = &skel->data_query->cpu_query;
+
+	hog = spawn_cpu_hog();
+	if (!ASSERT_GT(hog, 0, "spawn_cpu_hog"))
+		goto cleanup_link;
+
+	sleep(1);
+
+	/* Run the bpf program before anything here reads cpu.stat. */
+	if (!ASSERT_OK(read_stats(link), "read stats"))
+		goto cleanup_hog;
+
+	have_bw = wrote_max && cgroup_has_bw_stat(cgroup_fd);
+
+	if (test__start_subtest("cgroup_iter_cpu__cputime")) {
+		ASSERT_GT(q->usage_usec, 0, "usage_usec");
+		ASSERT_GT(q->user_usec + q->system_usec, 0, "user+system_usec");
+	}
+	if (test__start_subtest("cgroup_iter_cpu__throttling")) {
+		if (!have_bw) {
+			test__skip();
+		} else {
+			ASSERT_GT(q->nr_periods, 0, "nr_periods");
+			ASSERT_GT(q->nr_throttled, 0, "nr_throttled");
+			ASSERT_GT(q->throttled_usec, 0, "throttled_usec");
+			ASSERT_GT(q->throttled_self_usec, 0, "throttled_self_usec");
+		}
+	}
+
+	/*
+	 * cpu.stat cputime grows on every tick a task in the cgroup runs, so
+	 * stop them all before comparing
+	 */
+	if (test__start_subtest("cgroup_iter_cpu__match")) {
+		struct cpu_query filev = {};
+		int i, stable = 0;
+
+		kill(hog, SIGSTOP);
+		waitpid(hog, NULL, WUNTRACED);
+		if (!ASSERT_OK(join_root_cgroup(), "join_root_cgroup"))
+			goto cleanup_hog;
+
+		/*
+		 * The period timer keeps adding to nr_periods for a while
+		 * after the hog stops
+		 */
+		for (i = 0; i < 20; i++) {
+			struct cpu_query before = {}, after = {};
+
+			if (!ASSERT_OK(parse_stats(cgroup_fd, &before, have_bw), "cpu.stat") ||
+			    !ASSERT_OK(read_stats(link), "read stats") ||
+			    !ASSERT_OK(parse_stats(cgroup_fd, &after, have_bw), "cpu.stat"))
+				goto cleanup_hog;
+
+			if (!memcmp(&before, &after, sizeof(before))) {
+				filev = before;
+				stable = 1;
+				break;
+			}
+			usleep(100000);
+		}
+
+		if (!ASSERT_TRUE(stable, "cpu.stat stable"))
+			goto cleanup_hog;
+
+		ASSERT_EQ(q->usage_usec, filev.usage_usec, "usage_usec");
+		ASSERT_EQ(q->user_usec, filev.user_usec, "user_usec");
+		ASSERT_EQ(q->system_usec, filev.system_usec, "system_usec");
+		ASSERT_EQ(q->nice_usec, filev.nice_usec, "nice_usec");
+		ASSERT_EQ(q->forceidle_usec, filev.forceidle_usec, "forceidle_usec");
+
+		if (have_bw) {
+			ASSERT_EQ(q->nr_periods, filev.nr_periods, "nr_periods");
+			ASSERT_EQ(q->nr_throttled, filev.nr_throttled, "nr_throttled");
+			ASSERT_EQ(q->throttled_usec, filev.throttled_usec, "throttled_usec");
+			ASSERT_EQ(q->nr_bursts, filev.nr_bursts, "nr_bursts");
+			ASSERT_EQ(q->burst_usec, filev.burst_usec, "burst_usec");
+			ASSERT_EQ(q->throttled_self_usec, filev.throttled_self_usec,
+				  "throttled_self_usec");
+		}
+	}
+
+cleanup_hog:
+	kill(hog, SIGKILL);
+	waitpid(hog, NULL, 0);
+cleanup_link:
+	bpf_link__destroy(link);
+cleanup_skel:
+	cgroup_iter_cpu__destroy(skel);
+cleanup_cgroup_fd:
+	close(cgroup_fd);
+	cleanup_cgroup_environment();
+}
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
index 000000000000..6a288a00c25a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cgroup_iter_cpu.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include "cgroup_iter_cpu.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct cpu_query cpu_query SEC(".data.query");
+
+SEC("iter.s/cgroup")
+int cgroup_cpu_query(struct bpf_iter__cgroup *ctx)
+{
+	struct cpu_cgroup_cputime ct = {};
+	struct cgroup *cgrp = ctx->cgroup;
+	struct cgroup_subsys_state *css;
+	struct task_group *tg;
+
+	if (!cgrp)
+		return 1;
+
+	bpf_cpu_cgroup_flush_stats(cgrp);
+	bpf_cpu_cgroup_cputime(cgrp, &ct);
+
+	cpu_query.usage_usec = ct.usage_usec;
+	cpu_query.user_usec = ct.user_usec;
+	cpu_query.system_usec = ct.system_usec;
+	cpu_query.nice_usec = ct.nice_usec;
+	cpu_query.forceidle_usec = ct.forceidle_usec;
+
+	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();
+
+	/* a sum over every possible cpu, so the test uses a kfunc */
+	cpu_query.throttled_self_usec = bpf_cpu_cgroup_throttled_self(cgrp);
+
+	return 0;
+}
-- 
2.53.0-Meta

Re: [PATCH 2/2] selftests/bpf: add cgroup_iter_cpu test for cpu cgroup kfuncs

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 6799 bytes --]

> 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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31735302699

Re: [PATCH 1/2] cgroup, sched: add BPF kfuncs to read a cpu cgroup's stats

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4408 bytes --]

> 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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31735302699