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

[bot+bpf-ci@kernel.org]

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

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

> 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