[PATCH v2 0/2] block: expose blkcg io.stat to BPF

[PATCH v2 0/2] block: expose blkcg io.stat to BPF

[Ziyang Men]

Collecting cgroup statistics is expensive: the existing method is to
open and parse a cgroup file for every cgroup of interest. The memory
controller already has an efficient alternative through BPF; this series
extends that model to the block I/O controller's per-device io.stat.

Patch 1 adds the blkcg flush and per-device iterator.

Patch 2 adds a selftest that performs direct I/O on a private loop
device and compares every counter read by BPF with io.stat.

===
Changes since v1.

- Drop the blkcg acquire/release helpers. The program reads the io
  controller's css from cgroup->subsys[] under RCU instead.
- Remove the kfuncs that read the blkg device ID and counters; BPF
  programs can read those fields with BPF_CORE_READ().
- Keep the flush sleepable. It now takes a struct cgroup * and pins its
  CSS internally.
- Make the blkg iterator take an RCU-protected CSS.

Tested on VM with v7.2-rc5. 

Ziyang Men (2):
  block: add BPF kfuncs to read blkcg io.stat
  selftests/bpf: add test for blkcg io.stat BPF kfuncs

 MAINTAINERS                                   |   1 +
 block/Makefile                                |   3 +
 block/blk-cgroup.c                            |   2 +-
 block/blk-cgroup.h                            |   1 +
 block/bpf_blkcg.c                             | 154 ++++++++++
 tools/testing/selftests/bpf/cgroup_iter_io.h  |  17 ++
 tools/testing/selftests/bpf/config            |   1 +
 .../selftests/bpf/prog_tests/cgroup_iter_io.c | 277 ++++++++++++++++++
 .../selftests/bpf/progs/cgroup_iter_io.c      |  99 +++++++
 9 files changed, 554 insertions(+), 1 deletion(-)
 create mode 100644 block/bpf_blkcg.c
 create mode 100644 tools/testing/selftests/bpf/cgroup_iter_io.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
 create mode 100644 tools/testing/selftests/bpf/progs/cgroup_iter_io.c

-- 
2.53.0-Meta

[PATCH v2 1/2] block: add BPF kfuncs to read blkcg io.stat

[Ziyang Men]

Collecting cgroup statistics is expensive: the existing method is to
open and parse a cgroup file for every cgroup of interest. memcg already
has an efficient alternative through BPF; this series extends that idea
to block.  This series exposes the block I/O controller's per-device
io.stat to BPF.

The flush is sleepable and takes a cgroup. It pins the I/O css before
leaving RCU, then flushes it.  The iterator takes the RCU-protected css
and remains block-specific because each block device has its own blkg.
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.

Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 MAINTAINERS        |   1 +
 block/Makefile     |   3 +
 block/blk-cgroup.c |   2 +-
 block/blk-cgroup.h |   1 +
 block/bpf_blkcg.c  | 154 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100644 block/bpf_blkcg.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f9472c1a090..87c56e955577 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6617,6 +6617,7 @@ F:	block/blk-cgroup.c
 F:	block/blk-iocost.c
 F:	block/blk-iolatency.c
 F:	block/blk-throttle.c
+F:	block/bpf_blkcg.c
 F:	include/linux/blk-cgroup.h
 
 CONTROL GROUP - CPUSET
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/blk-cgroup.c b/block/blk-cgroup.c
index d9676126c5b5..8d538ad4e861 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1086,7 +1086,7 @@ static void blkcg_rstat_flush(struct cgroup_subsys_state *css, int cpu)
  * flushing the root cgroup's stats by explicitly filling in the iostat
  * with disk level statistics.
  */
-static void blkcg_fill_root_iostats(void)
+void blkcg_fill_root_iostats(void)
 {
 	struct class_dev_iter iter;
 	struct device *dev;
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 615390f751aa..8c9c2a1adfaa 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -205,6 +205,7 @@ void blkcg_deactivate_policy(struct gendisk *disk,
 			     const struct blkcg_policy *pol);
 
 const char *blkg_dev_name(struct blkcg_gq *blkg);
+void blkcg_fill_root_iostats(void);
 void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
 		       u64 (*prfill)(struct seq_file *,
 				     struct blkg_policy_data *, int),
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);
+}
+
+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);
-- 
2.53.0-Meta

[PATCH v2 2/2] selftests/bpf: add test for blkcg io.stat BPF kfuncs

[Ziyang Men]

Add cgroup_iter_io to test the blkcg io.stat BPF kfuncs. The BPF program
flushes the statistics, looks up the I/O css under RCU, walks its blkgs,
and reads one device's counters with BPF_CORE_READ().

The test performs O_DIRECT I/O on a private loop device. It checks the
device ID and counters against io.stat.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 tools/testing/selftests/bpf/cgroup_iter_io.h  |  17 ++
 tools/testing/selftests/bpf/config            |   1 +
 .../selftests/bpf/prog_tests/cgroup_iter_io.c | 277 ++++++++++++++++++
 .../selftests/bpf/progs/cgroup_iter_io.c      |  99 +++++++
 4 files changed, 394 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/cgroup_iter_io.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
 create mode 100644 tools/testing/selftests/bpf/progs/cgroup_iter_io.c

diff --git a/tools/testing/selftests/bpf/cgroup_iter_io.h b/tools/testing/selftests/bpf/cgroup_iter_io.h
new file mode 100644
index 000000000000..f4bbaaccdf71
--- /dev/null
+++ b/tools/testing/selftests/bpf/cgroup_iter_io.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#ifndef __CGROUP_ITER_IO_H
+#define __CGROUP_ITER_IO_H
+
+struct io_query {
+	/* one device's io.stat counters */
+	__u64 rbytes;
+	__u64 wbytes;
+	__u64 rios;
+	__u64 wios;
+	__u64 dbytes;
+	__u64 dios;
+	__u64 dev;	/* dev_t of the device the counters belong to */
+};
+
+#endif /* __CGROUP_ITER_IO_H */
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index ea7044f30adc..270e6bf9194d 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -1,3 +1,4 @@
+CONFIG_BLK_CGROUP=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BOOTPARAM_HARDLOCKUP_PANIC=y
 CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=1
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
new file mode 100644
index 000000000000..5d27e5d28379
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+#define _GNU_SOURCE
+#include <test_progs.h>
+#include <bpf/libbpf.h>
+#include <fcntl.h>
+#include <linux/loop.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <unistd.h>
+#include "cgroup_helpers.h"
+#include "cgroup_iter_io.h"
+#include "cgroup_iter_io.skel.h"
+
+#define IO_SIZE (4 * 1024 * 1024)
+
+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;
+
+	/* Results land in skel->data_query; the read itself returns no data. */
+	bytes = read(fd, NULL, 0);
+	if (!ASSERT_EQ(bytes, 0, "read fd"))
+		ret = 1;
+
+	close(fd);
+	return ret;
+}
+
+/* Set up a loop device for cgroup-charged I/O. */
+static int loop_setup(char *loop_path, size_t sz, int *ctl_fd, int *loop_fd,
+		      int *back_fd)
+{
+	char back_path[] = "/tmp/cgroup_iter_io.XXXXXX";
+	int nr;
+
+	*ctl_fd = *loop_fd = *back_fd = -1;
+
+	*ctl_fd = open("/dev/loop-control", O_RDWR | O_CLOEXEC);
+	if (*ctl_fd < 0)
+		return -1;
+
+	nr = ioctl(*ctl_fd, LOOP_CTL_GET_FREE);
+	if (nr < 0)
+		goto err;
+	snprintf(loop_path, sz, "/dev/loop%d", nr);
+
+	*back_fd = mkstemp(back_path);
+	if (*back_fd < 0)
+		goto err;
+	unlink(back_path);
+	if (ftruncate(*back_fd, (off_t)IO_SIZE * 4))
+		goto err;
+
+	*loop_fd = open(loop_path, O_RDWR | O_CLOEXEC);
+	if (*loop_fd < 0)
+		goto err;
+	if (ioctl(*loop_fd, LOOP_SET_FD, *back_fd))
+		goto err;
+
+	return 0;
+err:
+	if (*loop_fd >= 0)
+		close(*loop_fd);
+	if (*back_fd >= 0)
+		close(*back_fd);
+	close(*ctl_fd);
+	*ctl_fd = *loop_fd = *back_fd = -1;
+	return -1;
+}
+
+static void loop_teardown(const char *loop_path, int ctl_fd, int loop_fd,
+			  int back_fd)
+{
+	int nr = -1;
+
+	if (loop_fd >= 0) {
+		ioctl(loop_fd, LOOP_CLR_FD, 0);
+		close(loop_fd);
+	}
+	if (back_fd >= 0)
+		close(back_fd);
+	if (ctl_fd >= 0) {
+		if (sscanf(loop_path, "/dev/loop%d", &nr) == 1 && nr >= 0)
+			ioctl(ctl_fd, LOOP_CTL_REMOVE, nr);
+		close(ctl_fd);
+	}
+}
+
+/* O_DIRECT keeps I/O charged to the current cgroup. */
+static int do_direct_io(const char *loop_path)
+{
+	void *buf;
+	int fd, ret = -1;
+
+	fd = open(loop_path, O_RDWR | O_DIRECT | O_CLOEXEC);
+	if (fd < 0)
+		return -1;
+	if (posix_memalign(&buf, 4096, IO_SIZE))
+		goto out_fd;
+	memset(buf, 0xab, IO_SIZE);
+
+	if (pwrite(fd, buf, IO_SIZE, 0) != IO_SIZE)
+		goto out_buf;
+	fsync(fd);
+	if (pread(fd, buf, IO_SIZE, 0) != IO_SIZE)
+		goto out_buf;
+	ret = 0;
+out_buf:
+	free(buf);
+out_fd:
+	close(fd);
+	return ret;
+}
+
+/* Read @dev's io.stat counters. @dev uses kernel dev_t encoding. */
+static int parse_io_stat(int cgroup_fd, __u64 dev, struct io_query *out)
+{
+	unsigned int want_maj = dev >> 20, want_min = dev & ((1U << 20) - 1);
+	char buf[4096], *line, *saveptr;
+	int fd, n, ret = -1;
+
+	fd = openat(cgroup_fd, "io.stat", O_RDONLY);
+	if (fd < 0)
+		return -1;
+	n = read(fd, buf, sizeof(buf) - 1);
+	close(fd);
+	if (n <= 0)
+		return -1;
+	buf[n] = '\0';
+
+	for (line = strtok_r(buf, "\n", &saveptr); line;
+	     line = strtok_r(NULL, "\n", &saveptr)) {
+		unsigned long long rb = 0, wb = 0, ri = 0, wi = 0, db = 0, di = 0;
+		unsigned int maj, min;
+
+		/* Only the device id is required; missing counters stay zero. */
+		if (sscanf(line,
+			   "%u:%u rbytes=%llu wbytes=%llu rios=%llu wios=%llu dbytes=%llu dios=%llu",
+			   &maj, &min, &rb, &wb, &ri, &wi, &db, &di) < 2)
+			continue;
+		if (maj != want_maj || min != want_min)
+			continue;
+
+		out->rbytes = rb;
+		out->wbytes = wb;
+		out->rios = ri;
+		out->wios = wi;
+		out->dbytes = db;
+		out->dios = di;
+		ret = 0;
+		break;
+	}
+	return ret;
+}
+
+void test_cgroup_iter_io(void)
+{
+	char *cgroup_rel_path = "/cgroup_iter_io_test";
+	int ctl_fd = -1, loop_fd = -1, back_fd = -1;
+	struct cgroup_iter_io *skel = NULL;
+	struct bpf_link *link = NULL;
+	char loop_path[64];
+	struct io_query *q;
+	int cgroup_fd;
+
+	cgroup_fd = cgroup_setup_and_join(cgroup_rel_path);
+	if (!ASSERT_OK_FD(cgroup_fd, "cgroup_setup_and_join"))
+		return;
+
+	if (loop_setup(loop_path, sizeof(loop_path), &ctl_fd, &loop_fd, &back_fd)) {
+		test__skip();	/* needs root + CONFIG_BLK_DEV_LOOP */
+		goto cleanup_cgroup_fd;
+	}
+
+	skel = cgroup_iter_io__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "cgroup_iter_io__open_and_load"))
+		goto cleanup_loop;
+
+	/* Convert glibc st_rdev to kernel dev_t format. */
+	{
+		struct stat lst;
+
+		if (!ASSERT_OK(fstat(loop_fd, &lst), "fstat loop"))
+			goto cleanup_skel;
+		skel->data_query->target_dev =
+			((__u64)major(lst.st_rdev) << 20) | minor(lst.st_rdev);
+	}
+
+	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_io_query, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_iter"))
+		goto cleanup_skel;
+
+	/* This process is in the test cgroup, so the loop I/O is charged here. */
+	if (!ASSERT_OK(do_direct_io(loop_path), "do_direct_io"))
+		goto cleanup_link;
+
+	if (!ASSERT_OK(read_stats(link), "read stats"))
+		goto cleanup_link;
+
+	q = &skel->data_query->io_query;
+	if (test__start_subtest("cgroup_iter_io__write")) {
+		ASSERT_GT(q->wbytes, 0, "wbytes");
+		ASSERT_GT(q->wios, 0, "wios");
+	}
+	if (test__start_subtest("cgroup_iter_io__read")) {
+		ASSERT_GT(q->rbytes, 0, "rbytes");
+		ASSERT_GT(q->rios, 0, "rios");
+	}
+	if (test__start_subtest("cgroup_iter_io__dev"))
+		ASSERT_GT(q->dev, 0, "dev");
+
+	/* Compare with io.stat without I/O between the reads. */
+	if (test__start_subtest("cgroup_iter_io__match")) {
+		struct io_query filev = {};
+
+		if (ASSERT_OK(read_stats(link), "read stats") &&
+		    ASSERT_OK(parse_io_stat(cgroup_fd, q->dev, &filev),
+			      "parse io.stat")) {
+			ASSERT_EQ(q->rbytes, filev.rbytes, "rbytes");
+			ASSERT_EQ(q->wbytes, filev.wbytes, "wbytes");
+			ASSERT_EQ(q->rios, filev.rios, "rios");
+			ASSERT_EQ(q->wios, filev.wios, "wios");
+			ASSERT_EQ(q->dbytes, filev.dbytes, "dbytes");
+			ASSERT_EQ(q->dios, filev.dios, "dios");
+		}
+	}
+
+	/* Root statistics include this cgroup's I/O. */
+	if (test__start_subtest("cgroup_iter_io__root")) {
+		struct bpf_link *root_link;
+		struct io_query *r;
+
+		skel->data_query->got_root_css = 0;
+		root_link = bpf_program__attach_iter(skel->progs.cgroup_root_io_query,
+						     &opts);
+		if (ASSERT_OK_PTR(root_link, "attach root iter")) {
+			if (ASSERT_OK(read_stats(root_link), "read root stats")) {
+				r = &skel->data_query->root_query;
+				ASSERT_EQ(skel->data_query->got_root_css, 1,
+					  "got_root_css");
+				ASSERT_EQ(r->dev, q->dev, "root dev");
+				ASSERT_GE(r->wbytes, q->wbytes, "root wbytes");
+				ASSERT_GE(r->wios, q->wios, "root wios");
+				ASSERT_GE(r->rbytes, q->rbytes, "root rbytes");
+				ASSERT_GE(r->rios, q->rios, "root rios");
+			}
+			bpf_link__destroy(root_link);
+		}
+	}
+
+cleanup_link:
+	bpf_link__destroy(link);
+cleanup_skel:
+	cgroup_iter_io__destroy(skel);
+cleanup_loop:
+	loop_teardown(loop_path, ctl_fd, loop_fd, back_fd);
+cleanup_cgroup_fd:
+	close(cgroup_fd);
+	cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/progs/cgroup_iter_io.c b/tools/testing/selftests/bpf/progs/cgroup_iter_io.c
new file mode 100644
index 000000000000..2cd538068987
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cgroup_iter_io.c
@@ -0,0 +1,99 @@
+// 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 "bpf_experimental.h"
+#include "cgroup_iter_io.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct io_query io_query SEC(".data.query");
+
+struct io_query root_query SEC(".data.query");
+
+__u64 got_root_css SEC(".data.query");
+
+/* Device selected by userspace in kernel dev_t format. */
+__u64 target_dev SEC(".data.query");
+
+/* Keep inline: RCU and open-coded iterators cannot cross a BPF call. */
+static __always_inline int read_target_dev(struct cgroup *cgrp,
+					   struct io_query *out)
+{
+	struct cgroup_subsys_state *css;
+	struct blkcg_gq *pos;
+	__u64 dev;
+	int ssid;
+
+	/* The flush can sleep, so run it before the RCU section. */
+	bpf_blkcg_flush_stats(cgrp);
+
+	bpf_rcu_read_lock();
+	ssid = bpf_core_enum_value(enum cgroup_subsys_id, io_cgrp_id);
+
+	/*
+	 * subsys[] is __rcu, so this read gives an RCU pointer the iterator
+	 * accepts. BPF_CORE_READ() would return a plain value instead.
+	 */
+	css = cgrp->subsys[ssid];
+	if (!css) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+
+	bpf_for_each(blkg, pos, css) {
+		dev = BPF_CORE_READ(pos, q, disk, part0, bd_dev);
+		if (dev != target_dev)
+			continue;
+
+		out->dev = dev;
+		out->rbytes = BPF_CORE_READ(pos, iostat.cur.bytes[BLKG_IOSTAT_READ]);
+		out->wbytes = BPF_CORE_READ(pos, iostat.cur.bytes[BLKG_IOSTAT_WRITE]);
+		out->rios = BPF_CORE_READ(pos, iostat.cur.ios[BLKG_IOSTAT_READ]);
+		out->wios = BPF_CORE_READ(pos, iostat.cur.ios[BLKG_IOSTAT_WRITE]);
+		out->dbytes = BPF_CORE_READ(pos, iostat.cur.bytes[BLKG_IOSTAT_DISCARD]);
+		out->dios = BPF_CORE_READ(pos, iostat.cur.ios[BLKG_IOSTAT_DISCARD]);
+		break;
+	}
+	bpf_rcu_read_unlock();
+	return 1;
+}
+
+SEC("iter.s/cgroup")
+int cgroup_io_query(struct bpf_iter__cgroup *ctx)
+{
+	struct cgroup *cgrp = ctx->cgroup;
+
+	if (!cgrp)
+		return 1;
+
+	/* Start fresh so a device that is not found stays all-zero. */
+	__builtin_memset(&io_query, 0, sizeof(io_query));
+
+	read_target_dev(cgrp, &io_query);
+	return 0;
+}
+
+SEC("iter.s/cgroup")
+int cgroup_root_io_query(struct bpf_iter__cgroup *ctx)
+{
+	struct cgroup *root;
+
+	if (!ctx->cgroup)
+		return 1;
+
+	__builtin_memset(&root_query, 0, sizeof(root_query));
+
+	/* The root cgroup always has id 1. */
+	root = bpf_cgroup_from_id(1);
+	if (!root)
+		return 0;
+
+	/* Root counters include all cgroups' I/O. */
+	if (read_target_dev(root, &root_query))
+		got_root_css = 1;
+
+	bpf_cgroup_release(root);
+	return 0;
+}
-- 
2.53.0-Meta

Re: [PATCH v2 1/2] block: add BPF kfuncs to read blkcg io.stat

[bot+bpf-ci]

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

> 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

Re: [PATCH v2 2/2] selftests/bpf: add test for blkcg io.stat BPF kfuncs

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/cgroup_iter_io.h b/tools/testing/selftests/bpf/cgroup_iter_io.h
> new file mode 100644
> index 000000000000..f4bbaaccdf71
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/cgroup_iter_io.h

[ ... ]

> diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
> index ea7044f30adc..270e6bf9194d 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
> new file mode 100644
> index 000000000000..5d27e5d28379
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_io.c
> @@ -0,0 +1,277 @@

[ ... ]

> +/* Set up a loop device for cgroup-charged I/O. */
> +static int loop_setup(char *loop_path, size_t sz, int *ctl_fd, int *loop_fd,
> +		      int *back_fd)
> +{
> +	char back_path[] = "/tmp/cgroup_iter_io.XXXXXX";
> +	int nr;
> +
> +	*ctl_fd = *loop_fd = *back_fd = -1;
> +
> +	*ctl_fd = open("/dev/loop-control", O_RDWR | O_CLOEXEC);
> +	if (*ctl_fd < 0)
> +		return -1;
> +
> +	nr = ioctl(*ctl_fd, LOOP_CTL_GET_FREE);
> +	if (nr < 0)
> +		goto err;
> +	snprintf(loop_path, sz, "/dev/loop%d", nr);
> +
> +	*back_fd = mkstemp(back_path);
> +	if (*back_fd < 0)
> +		goto err;
> +	unlink(back_path);
> +	if (ftruncate(*back_fd, (off_t)IO_SIZE * 4))
> +		goto err;
> +
> +	*loop_fd = open(loop_path, O_RDWR | O_CLOEXEC);
> +	if (*loop_fd < 0)
> +		goto err;
> +	if (ioctl(*loop_fd, LOOP_SET_FD, *back_fd))
> +		goto err;
> +
> +	return 0;
> +err:
> +	if (*loop_fd >= 0)
> +		close(*loop_fd);
> +	if (*back_fd >= 0)
> +		close(*back_fd);
> +	close(*ctl_fd);
> +	*ctl_fd = *loop_fd = *back_fd = -1;
> +	return -1;
> +}

Does the err: cleanup path leak the loop device?

LOOP_CTL_GET_FREE can allocate a new loop device when no unbound device
exists, and nr is stored. Every failure after that point (mkstemp,
ftruncate, open(/dev/loopN), LOOP_SET_FD) leaves that device behind.

The caller jumps straight to cleanup_cgroup_fd on loop_setup() failure,
bypassing cleanup_loop, so loop_teardown() never runs and the device
remains allocated.

Recording nr and calling ioctl(*ctl_fd, LOOP_CTL_REMOVE, nr) on the err:
path would fix this. Additionally, loop_teardown() ignores the return of
both LOOP_CLR_FD and LOOP_CTL_REMOVE, so a device left in Lo_bound state
also leaks silently.

> +/* Read @dev's io.stat counters. @dev uses kernel dev_t encoding. */
> +static int parse_io_stat(int cgroup_fd, __u64 dev, struct io_query *out)
> +{
> +	unsigned int want_maj = dev >> 20, want_min = dev & ((1U << 20) - 1);
> +	char buf[4096], *line, *saveptr;
> +	int fd, n, ret = -1;
> +
> +	fd = openat(cgroup_fd, "io.stat", O_RDONLY);
> +	if (fd < 0)
> +		return -1;
> +	n = read(fd, buf, sizeof(buf) - 1);
> +	close(fd);
> +	if (n <= 0)
> +		return -1;
> +	buf[n] = '\0';

Will a single 4095-byte read() always capture the complete io.stat output?

io.stat is a seq_file with one line per blkg, and seq_read_iter() copies
with byte granularity, not record boundaries. Two failure modes:

1. If the cgroup's blkg list produces more than 4095 bytes, the loop
device's line may not be in the buffer, and parse_io_stat() returns -1.

2. The cut can land mid-line. A truncated line like "7:0 rbytes=4194304
wbytes=41" satisfies the '< 2' check below and is accepted with the
remaining counters silently zero, producing a bogus counter mismatch
against the kfunc rather than a parse error.

Reading in a loop until read() returns 0, or at least treating a non-
terminated final line as an error, would avoid both.

> +	for (line = strtok_r(buf, "\n", &saveptr); line;
> +	     line = strtok_r(NULL, "\n", &saveptr)) {
> +		unsigned long long rb = 0, wb = 0, ri = 0, wi = 0, db = 0, di = 0;
> +		unsigned int maj, min;
> +
> +		/* Only the device id is required; missing counters stay zero. */
> +		if (sscanf(line,
> +			   "%u:%u rbytes=%llu wbytes=%llu rios=%llu wios=%llu dbytes=%llu dios=%llu",
> +			   &maj, &min, &rb, &wb, &ri, &wi, &db, &di) < 2)
> +			continue;
> +		if (maj != want_maj || min != want_min)
> +			continue;
> +
> +		out->rbytes = rb;
> +		out->wbytes = wb;
> +		out->rios = ri;
> +		out->wios = wi;
> +		out->dbytes = db;
> +		out->dios = di;
> +		ret = 0;
> +		break;
> +	}
> +	return ret;
> +}

Should the partial conversion check be stricter?

Accepting '< 2' and then returning success means a line that matched the
device but whose counters were not parsed is reported as all-zero rather
than an error. The caller immediately uses strict equality checks:

	ASSERT_EQ(q->dbytes, filev.dbytes, "dbytes");

so a parse problem is reported as a counter mismatch against the kfunc,
pointing at the kernel instead of the parser.

This is reachable because blkcg_print_one_stat() emits the device name
unconditionally but gates the counter group with:

	if (rbytes || wbytes || rios || wios)
		seq_printf(s, "rbytes=%llu wbytes=%llu ... dios=%llu", ...);

A cgroup whose only traffic to a device was discard produces "MAJ:MIN "
with no key=value pairs. sscanf() returns 2, parse_io_stat() returns
success with dbytes=dios=0, and the match subtest fails on "dbytes" even
though the kernel is correct.

Requiring all eight conversions (== 8) once maj:min matches would surface
the real problem.

[ ... ]

> +	if (loop_setup(loop_path, sizeof(loop_path), &ctl_fd, &loop_fd, &back_fd)) {
> +		test__skip();	/* needs root + CONFIG_BLK_DEV_LOOP */
> +		goto cleanup_cgroup_fd;
> +	}

A subsystem pattern flags this as potentially concerning:

loop_setup() collapses eight distinct failures into a single -1, and the
caller turns every one of them into a bare test__skip() with no runtime
reason. The source comment says "needs root + CONFIG_BLK_DEV_LOOP", but
the same return value is produced by: open("/dev/loop-control") failing,
LOOP_CTL_GET_FREE failing, mkstemp() failing, ftruncate() failing (ENOSPC
on a small tmpfs /tmp), open("/dev/loopN") failing, and LOOP_SET_FD
failing.

The LOOP_SET_FD case is a genuine race: LOOP_CTL_GET_FREE is not a
reservation, so any concurrent consumer (another test_progs worker under
-j, udev, systemd, losetup) can bind /dev/loopN between GET_FREE and
SET_FD, and SET_FD then returns EBUSY. The result is that the whole test,
including all five subtests exercising the new blkcg kfuncs, silently
reports SKIP. Since nothing is printed, CI shows a green skip and a broken
kfunc goes unnoticed indefinitely.

Could the errno be printed before skipping, and "prerequisite absent"
(ENOENT/ENODEV/EPERM on /dev/loop-control) be distinguished from a real
failure (EBUSY, ENOSPC, ENOMEM)?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/cgroup_iter_io.c b/tools/testing/selftests/bpf/progs/cgroup_iter_io.c
> new file mode 100644
> index 000000000000..2cd538068987
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/cgroup_iter_io.c

[ ... ]


---
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