[PATCH v5 0/2] selftests/cgroup: fixes for test_zswap on single core VM

[PATCH v5 0/2] selftests/cgroup: fixes for test_zswap on single core VM

[Wilson Felipe Pereira]

This series fixes two test failures in test_zswap observed when running on
a single-core VM (-smp 1) with 4GB of RAM.

Patch 1 addresses a race condition in test_zswap_writeback() where
waitpid() returns before the exiting child process is switched away by the
kernel, causing an immediate write of "+memory" to cgroup.subtree_control
to fail with -EBUSY. We fix this by waiting for cgroup.events to report
"populated 0".

Patch 2 fixes an implicit unsigned conversion bug in test_no_kmem_bypass()
where small negative timing differences between debugfs stored_pages and
cgroup zswapped bytes caused the comparison to falsely fail due to
unsigned promotion.

v4 -> v5:
- Patch 2: Introduce `cg_read_key_long_long()` to return a `long long`
  value, and have `cg_read_key_long` use it and convert to `long`.
- Patch 2: Make `zswapped` a `long long`. This prevents a overflow on
  32-bit platforms (Andrew Morton, Sashiko).

v3 -> v4:
- Patch 2: Use `long long` for `delta` and `max_delta` and explicitly cast
  `stored_pages` to avoid integer overflow and implicit unsigned promotion
  warnings on 32-bit platforms (Andrew Morton, Sashiko).

v2 -> v3:
- Patch 1: Collect Acked-by from Michal Koutný.
- Patch 2: Explicitly cast stored_pages to (long) in delta calculation and
  inline the return comparison (Michal Koutný).

v1 -> v2:
- Patch 1: Replace EBUSY retry loop with cg_read_strcmp_wait() waiting for
  cgroup.events "populated 0" (Michal Koutný).
- Patch 1: Clarify task lifecycle in commit description (Yosry Ahmed).
- Patch 2: Remove abs() and declare delta/zswapped as signed longs with a
  signed threshold comparison (Michal Koutný).
- Patch 2: Add Fixes tag (Michal Koutný).

v4: https://lore.kernel.org/all/20260828033741.2184560-1-wfelipe@google.com/
v3: https://lore.kernel.org/all/20260827034807.2822234-1-wfelipe@google.com/
v2: https://lore.kernel.org/all/20260824033533.2147900-1-wfelipe@google.com/
v1: https://lore.kernel.org/all/20260804042053.56940-1-wfelipe@google.com/

Wilson Felipe Pereira (2):
  selftests/cgroup: test_zswap: wait for cgroup to unpopulate in
    test_zswap_writeback
  selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in
    test_no_kmem_bypass

 tools/testing/selftests/cgroup/lib/cgroup_util.c       | 17 ++++++++++++-----
 tools/testing/selftests/cgroup/lib/include/cgroup_util.h |  1 +
 tools/testing/selftests/cgroup/test_zswap.c            | 13 ++++++++-----
 3 files changed, 21 insertions(+), 10 deletions(-)

--

[PATCH v5 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback

[Wilson Felipe Pereira]

When running test_zswap on a single-core VM (-smp 1) with 4GB of RAM,
test_zswap_writeback intermittently fails on the initial run after boot.

In test_zswap_writeback(), after waitpid() reaps the child process created
by test_zswap_writeback_one(), writing "+memory" to cgroup.subtree_control
can fail with -EBUSY. Under cgroup v2, enabling domain subtree controllers
is forbidden while any tasks remain in cgroup.procs.

When a child process exits, exit_notify() wakes the parent process,
allowing waitpid() to return immediately. However, the cgroup populated
task count (nr_populated_csets) is only decremented when the exiting
task is switched away via finish_task_switch() -> cgroup_task_dead(). On
single-core systems, the parent runs before the dead child has been
switched out, causing "+memory" to fail with -EBUSY if written immediately
after waitpid() returns.

Fix this by waiting for cgroup.events to report "populated 0\n" via
cg_read_strcmp_wait() before enabling subtree control.

Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com>
Acked-by: Michal Koutný <mkoutny@suse.com>
---
 tools/testing/selftests/cgroup/test_zswap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index 609c48f38524..8f2c9aa4776c 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -408,6 +408,8 @@ static int test_zswap_writeback(const char *root, bool wb)
 	 * Thus, the parent's setting shall be what's in effect. */
 	if (cg_write(test_group, "memory.zswap.max", "max"))
 		goto out;
+	if (cg_read_strcmp_wait(test_group, "cgroup.events", "populated 0\n"))
+		goto out;
 	if (cg_write(test_group, "cgroup.subtree_control", "+memory"))
 		goto out;
 
-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v5 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass

[Wilson Felipe Pereira]

In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is
checked against stored_pages * page_size / 4 to verify that the pages
pushed to zswap belong to the test memory cgroup.

Due to slight stat update timing differences, delta can evaluate to a small
negative number (e.g. -5MB out of 1GB). Because delta is declared as a
signed int and stored_pages is an unsigned size_t, C's usual arithmetic
conversions implicitly promote a negative delta to a large unsigned 64-bit
integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate
to 0 and fail the test.

Fix this by declaring zswapped and delta as signed long long and comparing
against a signed threshold, ensuring negative deltas correctly evaluate
to true.

Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test")
Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com>
Acked-by: Michal Koutný <mkoutny@suse.com>
---
 tools/testing/selftests/cgroup/lib/cgroup_util.c    |  9 +++++++--
 .../selftests/cgroup/lib/include/cgroup_util.h      |  1 +
 tools/testing/selftests/cgroup/test_zswap.c         | 13 ++++++++-----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 2596c12cd864..ce2dd36291f8 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -172,7 +172,7 @@ long cg_read_long_fd(int fd)
 	return atol(buf);
 }
 
-long cg_read_key_long(const char *cgroup, const char *control, const char *key)
+long long cg_read_key_long_long(const char *cgroup, const char *control, const char *key)
 {
 	char buf[BUF_SIZE];
 	char *ptr;
@@ -184,7 +184,12 @@ long cg_read_key_long(const char *cgroup, const char *control, const char *key)
 	if (!ptr)
 		return -1;
 
-	return atol(ptr + strlen(key));
+	return atoll(ptr + strlen(key));
+}
+
+long cg_read_key_long(const char *cgroup, const char *control, const char *key)
+{
+	return (long)cg_read_key_long_long(cgroup, control, key);
 }
 
 long cg_read_key_long_poll(const char *cgroup, const char *control,
diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index 5d39c709ac7a..13923534003c 100644
--- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -69,6 +69,7 @@ extern int cg_read_strstr(const char *cgroup, const char *control,
 			  const char *needle);
 extern long cg_read_long(const char *cgroup, const char *control);
 extern long cg_read_long_fd(int fd);
+long long cg_read_key_long_long(const char *cgroup, const char *control, const char *key);
 long cg_read_key_long(const char *cgroup, const char *control, const char *key);
 long cg_read_key_long_poll(const char *cgroup, const char *control,
 			   const char *key, long expected, int retries,
diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index 8f2c9aa4776c..11c67dde705f 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -630,11 +630,14 @@ static int test_no_kmem_bypass(const char *root)
 			break;
 		/* If memory was pushed to zswap, verify it belongs to memcg */
 		if (stored_pages > stored_pages_threshold) {
-			int zswapped = cg_read_key_long(test_group, "memory.stat", "zswapped ");
-			int delta = stored_pages * page_size - zswapped;
-			int result_ok = delta < stored_pages * page_size / 4;
-
-			ret = result_ok ? KSFT_PASS : KSFT_FAIL;
+			long long zswapped = cg_read_key_long_long(
+				test_group, "memory.stat", "zswapped ");
+			long long delta =
+				(long long)stored_pages * page_size - zswapped;
+			long long max_delta =
+				(long long)stored_pages * page_size / 4;
+
+			ret = (delta < max_delta) ? KSFT_PASS : KSFT_FAIL;
 			break;
 		}
 	}
-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH v5 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass

[Michal Koutný]

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

On Tue, Sep 01, 2026 at 05:22:28AM +0000, Wilson Felipe Pereira <wfelipe@google.com> wrote:
> In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is
> checked against stored_pages * page_size / 4 to verify that the pages
> pushed to zswap belong to the test memory cgroup.
> 
> Due to slight stat update timing differences, delta can evaluate to a small
> negative number (e.g. -5MB out of 1GB). Because delta is declared as a
> signed int and stored_pages is an unsigned size_t, C's usual arithmetic
> conversions implicitly promote a negative delta to a large unsigned 64-bit
> integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate
> to 0 and fail the test.
> 
> Fix this by declaring zswapped and delta as signed long long and comparing
> against a signed threshold, ensuring negative deltas correctly evaluate
> to true.
> 
> Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test")
> Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com>
> Acked-by: Michal Koutný <mkoutny@suse.com>

(The long long helper is a non-trivial change, so the ack should be
stripped on this version (unless you can convince me ;-).)

I'm afraid this bitness propagated too broadly.

The long vs long long difference is only relevant on 32b, right?
And the bit-width issue only appears because of the product, not the
values read from memory.stat [1].

I'd consider three ways forward:
a) Consolidate the helpers into one cg_read_key_s64() (same explicit size
   regardless of arch),
b) keep single arch-dependent helper and tackle product(s) only,
c) do not bother with 32b in these selftests (I have a hunch that
   they're not so relevant on such archs).

WDYT?

Michal


[1] At least the values derived from available memory, memory events
    could grow indefinitely. It's true that kernel uses explicit u64 for
    all of the entries.

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

Re: [PATCH v5 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass

[Wilson Felipe Pereira]

Michal Koutný <mkoutny@suse.com> writes:

> On Tue, Sep 01, 2026 at 05:22:28AM +0000, Wilson Felipe Pereira <wfelipe@google.com> wrote:
>> In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is
>> checked against stored_pages * page_size / 4 to verify that the pages
>> pushed to zswap belong to the test memory cgroup.
>>
>> Due to slight stat update timing differences, delta can evaluate to a small
>> negative number (e.g. -5MB out of 1GB). Because delta is declared as a
>> signed int and stored_pages is an unsigned size_t, C's usual arithmetic
>> conversions implicitly promote a negative delta to a large unsigned 64-bit
>> integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate
>> to 0 and fail the test.
>>
>> Fix this by declaring zswapped and delta as signed long long and comparing
>> against a signed threshold, ensuring negative deltas correctly evaluate
>> to true.
>>
>> Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test")
>> Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com>
>> Acked-by: Michal Koutný <mkoutny@suse.com>
>
> (The long long helper is a non-trivial change, so the ack should be
> stripped on this version (unless you can convince me ;-).)

I'm sorry for that! Thanks for the review and for the patience.

>
> I'm afraid this bitness propagated too broadly.
>
> The long vs long long difference is only relevant on 32b, right?
> And the bit-width issue only appears because of the product, not the
> values read from memory.stat [1].
>
> I'd consider three ways forward:
> a) Consolidate the helpers into one cg_read_key_s64() (same explicit size
>    regardless of arch),
> b) keep single arch-dependent helper and tackle product(s) only,
> c) do not bother with 32b in these selftests (I have a hunch that
>    they're not so relevant on such archs).
>
> WDYT?

I like this option a), any thoughts on the sequence here?

I thought the following steps:
1) Add cg_read_key_s64() here and make use of it;
2) In another patch (probably 3/3), replace all uses of cg_read_key_long
   by cg_read_key_s64. There are 41 uses currently.

I believe the tests could be flaky in 32b already (just a guess), at
least option a) would make all be more deterministic.

>
> Michal
>
>
> [1] At least the values derived from available memory, memory events
>     could grow indefinitely. It's true that kernel uses explicit u64 for
>     all of the entries.

Re: [PATCH v5 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass

[Michal Koutný]

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

On Tue, Sep 01, 2026 at 10:42:51PM +0000, Wilson Felipe Pereira <wfelipe@google.com> wrote:
> I like this option a), any thoughts on the sequence here?
> 
> I thought the following steps:
> 1) Add cg_read_key_s64() here and make use of it;
> 2) In another patch (probably 3/3), replace all uses of cg_read_key_long
>    by cg_read_key_s64. There are 41 uses currently.

Sounds good to me.

Michal

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