[PATCH v2] selftests/cgroup: account for zswap shrinker writeback

[PATCH v2] selftests/cgroup: account for zswap shrinker writeback

[Joshua Hahn]

The test_no_invasive_cgroup_shrink selftest checks that when a cgroup
has zswapped out more memory than memory.zswap.max, it does not
trigger writeback for other cgroups. To do this, it compares the
writeback count in a control cgroup and makes sure that it is 0,
and then checks the writeback count in an aggressor cgroup who does
expect to see writeback.

However, when the zswap shrinker is enabled, the victim cgroup can see
legitimate writebacks not triggered by the aggressor. In some Meta CI
tests, we have seen this failure mode happen.

Instead of checking that the victim cgroup has 0 writeback, compare the
writeback values before and after the aggressor runs and check that
the victim cgroup did not perform any additional writeback. Note that
this can still lead to probabilistic failures if writebacks take longer
than 5 seconds, but this should fix the systematic failure case.

Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
Reported-by: Krush Chavan <krushchavan@outlook.com>
Suggested-by: Nhat Pham <nphamcs@gmail.com>
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
v1 --> v2:
- Instead of skipping the test when the zswap shrinker is enabled,
  compare the victim's writeback counts before and after the aggressor
  runs, as Nhat suggested.

 tools/testing/selftests/cgroup/test_zswap.c | 28 +++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index 9c5bd503c3f73..1ac7790727757 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -20,6 +20,7 @@ static int page_size;
 
 #define PATH_ZSWAP "/sys/module/zswap"
 #define PATH_ZSWAP_ENABLED "/sys/module/zswap/parameters/enabled"
+#define PATH_ZSWAP_SHRINKER_ENABLED "/sys/module/zswap/parameters/shrinker_enabled"
 #define PATH_ZSWAP_STORED_PAGES "/sys/kernel/debug/zswap/stored_pages"
 
 static int read_int(const char *path, size_t *value)
@@ -446,6 +447,16 @@ static int test_zswap_writeback_disabled(const char *root)
 	return test_zswap_writeback(root, false);
 }
 
+static bool zswap_shrinker_enabled(void)
+{
+	char value[2];
+
+	if (read_text(PATH_ZSWAP_SHRINKER_ENABLED, value, sizeof(value)) <= 0)
+		return 0;
+
+	return value[0] == 'Y';
+}
+
 /*
  * When trying to store a memcg page in zswap, if the memcg hits its memory
  * limit in zswap, writeback should affect only the zswapped pages of that
@@ -455,6 +466,7 @@ static int test_no_invasive_cgroup_shrink(const char *root)
 {
 	int ret = KSFT_FAIL;
 	unsigned int off;
+	long zswpwb_before, zswpwb_after, zswpwb_target;
 	size_t allocation_size = page_size * 1024;
 	unsigned int nr_pages = allocation_size / page_size;
 	char zswap_max_buf[32], mem_max_buf[32];
@@ -490,6 +502,14 @@ static int test_no_invasive_cgroup_shrink(const char *root)
 	if (cg_read_key_long(zw_group, "memory.stat", "zswapped") < 1)
 		goto out;
 
+	/* If the shrinker is enabled, try to let the writebacks finish first */
+	if (zswap_shrinker_enabled())
+		sleep(5);
+
+	zswpwb_before = get_cg_wb_count(zw_group);
+	if (zswpwb_before < 0)
+		goto out;
+
 	/* Push wb_group memory into zswap with hard-to-compress data to trigger wb */
 	if (cg_enter_current(wb_group))
 		goto out;
@@ -502,9 +522,13 @@ static int test_no_invasive_cgroup_shrink(const char *root)
 		getrandom(&wb_allocation[off], page_size/4, 0);
 	}
 
-	/* Verify that only zswapped memory from gwb_group has been written back */
-	if (wait_for_writeback(wb_group, 5000) > 0 && get_cg_wb_count(zw_group) == 0)
+	/* Verify that only zswapped memory from wb_group has been written back */
+	zswpwb_target = wait_for_writeback(wb_group, 5000);
+	zswpwb_after = get_cg_wb_count(zw_group);
+
+	if (zswpwb_target > 0 && zswpwb_before == zswpwb_after)
 		ret = KSFT_PASS;
+
 out:
 	cg_enter_current(root);
 	if (zw_group) {
-- 
2.53.0-Meta

Re: [PATCH v2] selftests/cgroup: account for zswap shrinker writeback

[Andrew Morton]

On Wed,  2 Sep 2026 12:45:20 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> The test_no_invasive_cgroup_shrink selftest checks that when a cgroup
> has zswapped out more memory than memory.zswap.max, it does not
> trigger writeback for other cgroups. To do this, it compares the
> writeback count in a control cgroup and makes sure that it is 0,
> and then checks the writeback count in an aggressor cgroup who does
> expect to see writeback.
> 
> However, when the zswap shrinker is enabled, the victim cgroup can see
> legitimate writebacks not triggered by the aggressor. In some Meta CI
> tests, we have seen this failure mode happen.
> 
> Instead of checking that the victim cgroup has 0 writeback, compare the
> writeback values before and after the aggressor runs and check that
> the victim cgroup did not perform any additional writeback. Note that
> this can still lead to probabilistic failures if writebacks take longer
> than 5 seconds, but this should fix the systematic failure case.
> 
> Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")

Thanks.

I do support backporting selftests fixes.  We want selftests to work
well in the kernel with which they are shipped.  But, as ever, we
should include a clear explanation of the userspace-visible runtime
effects of the bug.

Seems the short answer is "false-positive test failures" but we can
perhaps do a little better than that.  Quoting the failure messages
would be a good way - very recognizable.

> Reported-by: Krush Chavan <krushchavan@outlook.com>

Relatedly, is there a Link:/Closes:?

Re: [PATCH v2] selftests/cgroup: account for zswap shrinker writeback

[Joshua Hahn]

On Wed, 2 Sep 2026 18:04:08 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed,  2 Sep 2026 12:45:20 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > The test_no_invasive_cgroup_shrink selftest checks that when a cgroup
> > has zswapped out more memory than memory.zswap.max, it does not
> > trigger writeback for other cgroups. To do this, it compares the
> > writeback count in a control cgroup and makes sure that it is 0,
> > and then checks the writeback count in an aggressor cgroup who does
> > expect to see writeback.
> > 
> > However, when the zswap shrinker is enabled, the victim cgroup can see
> > legitimate writebacks not triggered by the aggressor. In some Meta CI
> > tests, we have seen this failure mode happen.
> > 
> > Instead of checking that the victim cgroup has 0 writeback, compare the
> > writeback values before and after the aggressor runs and check that
> > the victim cgroup did not perform any additional writeback. Note that
> > this can still lead to probabilistic failures if writebacks take longer
> > than 5 seconds, but this should fix the systematic failure case.
> > 
> > Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")

Hello Andrew, I hope you're doing well!

> Thanks.
> 
> I do support backporting selftests fixes.  We want selftests to work
> well in the kernel with which they are shipped.  But, as ever, we
> should include a clear explanation of the userspace-visible runtime
> effects of the bug.
> 
> Seems the short answer is "false-positive test failures" but we can
> perhaps do a little better than that.  Quoting the failure messages
> would be a good way - very recognizable.

That sounds good to me. Unfortunately it seems like the selftests
aren't too expressive in terms of why it failed, so this is all that
I was able to pick up from the selftest logs:

ok 1 test_zswap_usage
ok 2 test_swapin_nozswap
ok 3 test_zswapin
ok 4 test_zswap_writeback_enabled
ok 5 test_zswap_writeback_disabled
ok 6 # SKIP test_no_kmem_bypass
not ok 7 test_no_invasive_cgroup_shrink
ok 8 test_zswap_incompressible
# 1 skipped test(s) detected. Consider enabling relevant config options to improve coverage.
# Totals: pass:6 fail:1 xfail:0 xpass:0 skip:1 error:0

I'm not too sure how helpful this will be, but maybe it would be better
to amend the last line of the commit message to say:

...
than 5 seconds, but this should fix the systematic failure case
and make "not ok test_no_invasive_cgroup_shrink" less likely.

> > Reported-by: Krush Chavan <krushchavan@outlook.com>
> 
> Relatedly, is there a Link:/Closes:?

Unfortunately this was detected in our private CI workflow and raised
by Krush, so I'm not sure if there is anything we can share.

If there is anything else that I can add to make this clearer, please
let me know. I hope you have a great day Andrew!
Joshua