Re: [RFC PATCH v12 3/4] selftests/bpf: add iocost cost model test

[Tao Cui]

Hello,

在 2026/11/3 22:07, bot+bpf-ci@xxxxxxxxxx 写道:
>> selftests/bpf: add iocost cost model test
> 
> TL;DR: Selftest cgroup names "iocost_pairing" and "iocost_ms" lack a leading
> '/', so the cgroups are likely created outside the work directory and not
> removed by cleanup_cgroup_environment(), leaving empty cgroups on the host.
> 
Confirmed. The missing leading '/' places both cgroups alongside the
per-test work directory instead of inside it, so
cleanup_cgroup_environment() never visits them. The test itself still
works because every helper builds the same path.

I'll switch every use to "/iocost_pairing" and "/iocost_ms", so the
cgroups live under the work directory and are removed by the existing
cleanup.

Thanks,
Tao
>> diff --git a/tools/testing/selftests/bpf/prog_tests/iocost_model.c b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
>> new file mode 100644
> 
> [ ... ]
> 
>> +	/*
>> +	 * cgroup_setup_and_join() creates the priced-IO cgroup with the
>> +	 * available controllers enabled, so the cgroup has
>> +	 * io.weight/io.stat and the iocg activates.
>> +	 * cleanup_cgroup_environment() removes it from every exit path
>> +	 * below. io.cost.qos and io.cost.model are root-only cgroupfs
>> +	 * files: they are written at the hierarchy root and reverted on
>> +	 * exit.
>> +	 */
>> +	cg = cgroup_setup_and_join("iocost_pairing");
> 
> [ ... ]
> 
>> +		cg = cgroup_setup_and_join("iocost_ms");
> 
> Will these cgroups actually be removed by cleanup_cgroup_environment()?
> 
> The name has no leading '/', and format_cgroup_path() in cgroup_helpers.c
> joins the pieces with no separator:
> 
>     #define format_cgroup_path_pid(buf, path, pid) \
>         snprintf(buf, sizeof(buf), "%s%s%d%s", CGROUP_MOUNT_PATH, \
>         CGROUP_WORK_DIR, pid, path)
> 
> So cgroup_setup_and_join("iocost_pairing") ends up doing mkdir on
> "/mnt/cgroup-test-work-dir<pid>iocost_pairing".  That is a sibling of the
> work directory directly under the cgroup2 root, not a child of it.
> join_cgroup(), write_cgroup_file() and read_cgroup_file() build the same
> path, so the test itself works.
> 
> But __cleanup_cgroup_environment() only walks the work directory:
> 
>     format_cgroup_path(cgroup_workdir, "");
>     join_cgroup_from_top(CGROUP_MOUNT_PATH);
>     nftw(cgroup_workdir, nftwfunc, WALK_FD_LIMIT, FTW_DEPTH | FTW_MOUNT);
> 
> so the sibling directory is never visited.  The empty cgroup remains on the
> host as /sys/fs/cgroup/cgroup-test-work-dir<pid>iocost_pairing, which also
> contradicts the comment above ("cleanup_cgroup_environment() removes it from
> every exit path below").  serial_test_iocost_model_streams() has the same
> problem with "iocost_ms", including the matching write_cgroup_file() and
> read_cost_usage() calls.
> 
> Other callers that use a fixed name pass a leading slash, for example
> "/skb_cgroup_test" and "/cgroup_iter_memcg_test".  Would "/iocost_pairing"
> and "/iocost_ms" at every place they are passed put the cgroups under the
> work directory where the existing cleanup removes them?
> 
> 
> ---
> 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/41646836440