> 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 <[email protected]>
> + */
> +
> +#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
