Re: [PATCH] selftests/cgroup: add PSI pressure trigger and validation tests

["Michal Koutný" <mkoutny@suse.com>]

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

Hi.

On Fri, Jul 24, 2026 at 10:58:26AM +0800, Tao Cui <cui.tao@linux.dev> wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> The cgroup selftest suite had no PSI coverage at all. Add test_psi.c:
> 
>   - /proc/pressure/{cpu,memory,io} trigger validation: a second trigger
>     on the same fd fails EBUSY; out-of-range window/threshold and
>     unparseable input fail EINVAL; "some" on /proc/pressure/irq fails
>     EINVAL (skipped when the file is absent).

If you want to test each and every returned error code, I'd suggest you
try adding this to LTP [1]. However, in general I don't think it's a
good idea to ossify the APIs with so strict tests.
The point of these selftests is for developers to have some quick
smoketests verifying that the functional concepts don't get broken while
having simple tests that are easy to maintain.


>   - cgroup.pressure toggle: disabling hides *.pressure files and
>     round-trips; out-of-range input fails ERANGE, non-numeric EINVAL.

This is a good test IMO, except for the too constrained range checking.


>   - best-effort positive test: arm a sensitive memory.pressure trigger
>     under a tight memory.max, churn memory to induce a stall and expect
>     poll() to wake (SKIP if no stall can be induced).

If you want to test the trigger, it might be better to pick a different
resource than memory (also why you sometimes cover all resource with
tests and sometimes pick only memory?), cpu pressure can be in my
experience (and in theory) induced much more predictably.

Also if you stick to memory & SKIP, could be expand on the reasons
behind selection of PSI_POLL_TIMEOUT_MS and the proportion of skip vs
pass results?



> +/* Touch every page to force allocation under a tight memory.max. */
> +static void churn_memory(size_t size)
> +{
> +	char *p = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	int iter;
> +
> +	if (p == MAP_FAILED)
> +		return;
> +	for (iter = 0; iter < 5; iter++) {
> +		size_t i;
> +
> +		for (i = 0; i < size; i += 4096)

This would ideally be sysconf(_SC_PAGE_SIZE) (see test_memcontrol.c).
Generally, this looks like something to extract and share across tests,
also check [2].

(But as I wrote above, memory is not a good resource to predictable
behavior of PSI triggers.)

> +int main(int argc, char **argv)
> +{
> +	char root[PATH_MAX];
> +	int i;
> +
> +	(void)argc;
> +
> +	ksft_print_header();
> +	ksft_set_plan(ARRAY_SIZE(tests));
> +
> +	if (cg_find_unified_root(root, sizeof(root), NULL))
> +		ksft_exit_skip("cgroup v2 isn't mounted\n");
> +	if (cg_read_strstr(root, "cgroup.controllers", "memory"))
> +		ksft_exit_skip("memory controller isn't available\n");
> +	if (cg_read_strstr(root, "cgroup.subtree_control", "memory"))
> +		if (cg_write(root, "cgroup.subtree_control", "+memory"))
> +			ksft_exit_skip("failed to enable memory controller\n");

Here'd be good some skip-guards so that the test doesn't fail on
CONFIG_PSI=n or CONFIG_PSI_DEFAULT_DISABLED=y kernels.

Similar for the IRQ test and CONFIG_IRQ_TIME_ACCOUNTING
(which additionally would be IMO more uniform to have the IRQ test
together with others and just skip the "some" part).

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]