> The behavior mirrows the blkcg_print_stat().
>
> The blkg device iterator take a RCU css.
>
> No kfuncs are added to read the blkcg counters since user can read it
> using the BPF_CORE_READ.
This isn't a bug, but there are a few rough spots in the changelog:
"mirrows" should be "mirrors", "The blkg device iterator take a RCU css."
reads as a fragment that restates the earlier sentence about the iterator
taking the RCU-protected css, and "since user can read it" is missing an
article before "user".
Could the changelog also say why a new flush kfunc is needed rather than
reusing the existing css_rstat_flush() kfunc -- is the point specifically
the root-cgroup blkcg_fill_root_iostats() path?
> diff --git a/block/Makefile b/block/Makefile
> index e7bd320e3d69..572e49988c8e 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -17,6 +17,9 @@ obj-$(CONFIG_BLK_ERROR_INJECTION) += error-injection.o
> obj-$(CONFIG_BLK_DEV_BSG_COMMON) += bsg.o
> obj-$(CONFIG_BLK_DEV_BSGLIB) += bsg-lib.o
> obj-$(CONFIG_BLK_CGROUP) += blk-cgroup.o
> +ifdef CONFIG_BPF_SYSCALL
> +obj-$(CONFIG_BLK_CGROUP) += bpf_blkcg.o
> +endif
> obj-$(CONFIG_BLK_CGROUP_RWSTAT) += blk-cgroup-rwstat.o
> obj-$(CONFIG_BLK_CGROUP_FC_APPID) += blk-cgroup-fc-appid.o
> obj-$(CONFIG_BLK_DEV_THROTTLING) += blk-throttle.o
[ ... ]
> diff --git a/block/bpf_blkcg.c b/block/bpf_blkcg.c
> new file mode 100644
> index 000000000000..25c809f5091c
> --- /dev/null
> +++ b/block/bpf_blkcg.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Block I/O Controller-related BPF kfuncs and auxiliary code
> + */
> +
> +#include "blk-cgroup.h"
> +
> +#include <linux/bpf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/rculist.h>
> +
> +__bpf_kfunc_start_defs();
> +
> +/**
> + * bpf_blkcg_flush_stats - Flush a block cgroup's io statistics
> + * @cgrp: cgroup to flush
> + *
> + * Propagate I/O statistics up the cgroup tree. Root statistics come from
> + * block devices and include all cgroups' I/O.
> + */
> +__bpf_kfunc void bpf_blkcg_flush_stats(struct cgroup *cgrp)
> +{
> + struct cgroup_subsys_state *css;
> +
> + /* Pin the css for the sleepable flush. */
> + rcu_read_lock();
> + css = cgroup_css(cgrp, &io_cgrp_subsys);
> + if (css && !css_tryget(css))
> + css = NULL;
> + rcu_read_unlock();
> +
> + if (!css)
> + return;
> +
> + if (!css->parent)
> + blkcg_fill_root_iostats();
> + else
> + css_rstat_flush(css);
> +
> + css_put(css);
> +}
This isn't a bug, but would it be simpler for bpf_blkcg_flush_stats() to
take the io css directly, so the same handle feeds bpf_iter_blkg_new()?
The two entry points in this file take different cgroup handles:
bpf_blkcg_flush_stats() takes struct cgroup* and resolves the css
internally with cgroup_css()/css_tryget(), while bpf_iter_blkg_new()
below takes struct cgroup_subsys_state* directly. A program that wants to
flush and then iterate therefore needs both handle types for the same
cgroup. Taking struct cgroup_subsys_state* in both, as mm/bpf_memcontrol.c
does for its memcg handles, would make them composable and would drop the
cgroup_css() lookup.
> +
> +struct bpf_iter_blkg {
> + __u64 __opaque[2];
> +} __aligned(8);
> +
> +struct bpf_iter_blkg_kern {
> + struct blkcg *blkcg;
> + struct blkcg_gq *pos;
> +} __aligned(8);
> +
> +/**
> + * bpf_iter_blkg_new - Start iterating a block cgroup's per-device blkgs
> + * @it: iterator to initialize
> + * @css: the io controller's css
> + *
> + * Each blkg holds one device's io.stat counters. Offline blkgs are skipped.
> + * A blkg without a disk can be returned. Must run under RCU.
> + *
> + * Return: 0 on success, -EINVAL if @css is not the io controller's.
> + */
> +__bpf_kfunc int bpf_iter_blkg_new(struct bpf_iter_blkg *it,
> + struct cgroup_subsys_state *css)
> +{
> + struct bpf_iter_blkg_kern *kit = (void *)it;
> +
> + BUILD_BUG_ON(sizeof(struct bpf_iter_blkg_kern) > sizeof(struct
> bpf_iter_blkg));
> + BUILD_BUG_ON(__alignof__(struct bpf_iter_blkg_kern) !=
> + __alignof__(struct bpf_iter_blkg));
> +
> + kit->pos = NULL;
> +
> + if (css->ss != &io_cgrp_subsys) {
> + kit->blkcg = NULL;
> + return -EINVAL;
> + }
> +
> + kit->blkcg = css_to_blkcg(css);
> + return 0;
> +}
> +
> +/**
> + * bpf_iter_blkg_next - Return the next online blkg of the iterated block
> cgroup
> + * @it: iterator
> + *
> + * Return: the next online blkg, or NULL when the walk is done.
> + */
> +__bpf_kfunc struct blkcg_gq *bpf_iter_blkg_next(struct bpf_iter_blkg *it)
> +{
> + struct bpf_iter_blkg_kern *kit = (void *)it;
> + struct blkcg_gq *blkg = kit->pos;
> + struct hlist_node *node;
> +
> + if (!kit->blkcg)
> + return NULL;
> +
> + if (!blkg)
> + node = rcu_dereference(hlist_first_rcu(&kit->blkcg->blkg_list));
> + else
> + node = rcu_dereference(hlist_next_rcu(&blkg->blkcg_node));
> +
> + /* Skip offline blkgs, matching io.stat. */
> + while (node) {
> + blkg = hlist_entry(node, struct blkcg_gq, blkcg_node);
> + /* A race only changes whether this blkg is returned. */
> + if (data_race(blkg->online)) {
> + kit->pos = blkg;
> + return blkg;
> + }
> + node = rcu_dereference(hlist_next_rcu(&blkg->blkcg_node));
> + }
> +
> + /* The iterator must keep returning NULL after completion. */
> + kit->pos = NULL;
> + kit->blkcg = NULL;
> + return NULL;
> +}
> +
> +/**
> + * bpf_iter_blkg_destroy - Tear down a blkg iterator
> + * @it: iterator
> + */
> +__bpf_kfunc void bpf_iter_blkg_destroy(struct bpf_iter_blkg *it)
> +{
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(bpf_blkcg_kfuncs)
> +BTF_ID_FLAGS(func, bpf_blkcg_flush_stats, KF_SLEEPABLE)
> +
> +BTF_ID_FLAGS(func, bpf_iter_blkg_new,
> + KF_ITER_NEW | KF_RCU | KF_RCU_PROTECTED)
> +BTF_ID_FLAGS(func, bpf_iter_blkg_next, KF_ITER_NEXT | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_iter_blkg_destroy, KF_ITER_DESTROY)
> +BTF_KFUNCS_END(bpf_blkcg_kfuncs)
> +
> +static const struct btf_kfunc_id_set bpf_blkcg_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &bpf_blkcg_kfuncs,
> +};
> +
> +static int __init bpf_blkcg_init(void)
> +{
> + int err;
> +
> + err = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
> + &bpf_blkcg_kfunc_set);
> + if (err)
> + pr_warn("error while registering bpf blkcg kfuncs: %d\n", err);
> +
> + return err;
> +}
> +late_initcall(bpf_blkcg_init);
This isn't a bug, but could a selftest under tools/testing/selftests/bpf/
be added alongside these kfuncs to lock down the RCU-protected iterator
usage? Four new kfuncs are registered for BPF_PROG_TYPE_UNSPEC (all
program types) with no accompanying selftest and no in-tree consumer, so
nothing exercises the iterator's KF_RCU_PROTECTED contract, the
offline-blkg skip, or the root-vs-non-root flush split. Comparable
additions -- mm/bpf_memcontrol.c and the bpf_iter_css family -- landed
with tests under tools/testing/selftests/bpf/, which is also what pins the
intended usage pattern (bpf_rcu_read_lock() around new/next/destroy) for
future readers.
---
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/32073368069
