Re: [PATCH 3/5] selftests/bpf: add iocost cost model test

[Tejun Heo <tj@kernel.org>]

Hello, Tao.

On Wed, Oct 28, 2026 at 10:35:32PM +0800, Tao Cui wrote:
> +static bool iocost_bpf_supported(void)
> +{
> +	FILE *f;
> +	char line[128];
> +	bool any = false, on = false;
> ...
> +		if (strstr(line, "CONFIG_"))
> +			any = true;
> ...
> +	return on;

any is set but never read, so every test skips without /proc/config.gz.

> +	/* isolate the configfs and cgroup mounts from the host */
> +	if (unshare(CLONE_NEWNS)) {

unshare(CLONE_NEWNS) alone doesn't make the mounts private, so the
configfs mount propagates to the host, one more per test.

> +	ASSERT_OK(configfs_write("/sys/fs/cgroup/cgroup.subtree_control",
> +				 "+io"), "subtree_io");
> +	if (!ASSERT_OK(cgroup_mkdir("/sys/fs/cgroup/iocost_pairing"),

The "+io" write to the root cgroup's cgroup.subtree_control is never
reverted, the cgroups are created directly under /sys/fs/cgroup and
test_progs stays in the root cgroup. cgroup_helpers handles all of this.

> +	err = iocost_model__attach(skel);
> +	if (err == -EOPNOTSUPP || err == -ENODEV) {
> ...
> +		test__skip();

-ENODEV and -EOPNOTSUPP from attaching to the test's own null_blk device
are failures, not skips. The same goes for
serial_test_iocost_model_streams().

> +		ASSERT_GT(skel->bss->iocg_inits, 0, "pair_init");

This passes on the root blkg alone, so it doesn't show that the test
cgroup got iocg_init(). The same goes for eject_had_inits and
pair_init_again.

> +		/*
> +		 * detach implicitly by closing the link fd instead of
> +		 * calling bpf_link__destroy() explicitly: the release
> +		 * path must detach the model just like the explicit
> +		 * call does
> +		 */
> +		lfd = bpf_link__fd(skel->links.iocost_2x);
> +		bpf_link__disconnect(skel->links.iocost_2x);
> +		close(lfd);
> +		skel->links.iocost_2x = NULL;

This leaks libbpf's link struct. The reason in the comment doesn't hold
either. bpf_link__destroy() on a struct_ops link does the same close().

> +void serial_test_iocost_model_streams(void)
> ...
> +	if (ASSERT_OK(err, "attach")) {
> +		err = readback_model(dev, model, sizeof(model));
> +		if (ASSERT_OK(err, "readback"))
> +			ASSERT_STREQ(model, "bpf", "model_bpf");
> +
> +		bpf_link__destroy(skel->links.iocost_ms);

The test never enables the controller, so iocost_ms's calc_cost() never
runs. The multi-stream results in the cover letter aren't exercised by
the test.

Thanks.

--
tejun