Re: [PATCH] mm: zswap: honor zero cgroup limits after successful reclaim

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] selftests/cgroup: missing wait for populated 0 in
  test_zswap_max_zero()
- [Low] selftests/cgroup: silent KSFT_SKIP returns in
  test_zswap_max_zero()
--

commit cb5209a5f058b3ca9ae23b219994b4a826f192b3
Author: Tomás Senart <ts@perfloop.ai>

mm: zswap: honor zero cgroup limits after successful reclaim

This commit checks for a zero zswap limit across the cgroup hierarchy after
reclaiming entries in zswap_store() so that a cgroup with memory.zswap.max
set to zero does not store newly compressed pages, and adds a selftest.

> diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
> index d50acc1b83ac8..7632aadd88662 100644
> --- a/tools/testing/selftests/cgroup/test_zswap.c
> +++ b/tools/testing/selftests/cgroup/test_zswap.c

[ ... ]

> @@ -244,6 +245,105 @@ static int test_swapin_nozswap(const char *root)
> +static int test_zswap_max_zero(const char *root)
> +{
> +	char *group = NULL, *child = NULL;
> +	int ret = KSFT_FAIL;
> +
> +	if (access(PATH_ZSWAP_STORED_PAGES, R_OK) ||
> +	    access(PATH_ZSWAP_WRITTEN_BACK_PAGES, R_OK))
> +		return KSFT_SKIP;
> +	if (cg_read_strcmp(root, "memory.zswap.writeback", "1"))
> +		return KSFT_SKIP;

[Severity: Low]
This isn't a bug, but returning KSFT_SKIP here without calling
ksft_print_msg() leaves out why the test was skipped, since main() only
passes the test name to ksft_test_result_skip().

> +
> +	group = cg_name(root, "zswap_max_zero_test");
> +	if (!group || cg_create(group))
> +		goto out;
> +	if (cg_run(group, pageout_zero_limit, group))
> +		goto out;
> +
> +	if (cg_write(group, "memory.zswap.max", "max"))
> +		goto out;
> +	if (cg_write(group, "cgroup.subtree_control", "+memory"))
> +		goto out;

[Severity: Low]
Can writing "+memory" to cgroup.subtree_control fail intermittently with
-EBUSY right after cg_run() returns?

In the exit path, exit_notify() wakes waitpid() in cg_run() before the
dying child completes its final context switch in finish_task_switch() ->
cgroup_task_dead(), which decrements nr_populated_csets.  If the parent
writes "+memory" to group/cgroup.subtree_control before that happens (such
as on a single-CPU VM), cgroup_vet_subtree_control_enable() sees
cgroup_has_tasks(cgrp) and rejects the write with -EBUSY, causing a false
test failure.

Wait for the cgroup to become unpopulated with
cg_read_strcmp_wait(group, "cgroup.events", "populated 0\n") before
enabling "+memory", as done in test_zswap_writeback() since commit
5547ee3a76b2 ("selftests/cgroup: test_zswap: wait for cgroup to unpopulate
in test_zswap_writeback").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-zswap-zero-limit-v1-1-19f33081b09e@perfloop.ai?part=1