Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] selftests/cgroup: fixes for test_zswap on single core VM
@ 2026-08-24  3:33 Wilson Felipe Pereira
  2026-08-24  3:33 ` [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback Wilson Felipe Pereira
  2026-08-24  3:33 ` [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
  0 siblings, 2 replies; 5+ messages in thread
From: Wilson Felipe Pereira @ 2026-08-24  3:33 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Yosry Ahmed, Nhat Pham,
	Chengming Zhou, Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, linux-kernel,
	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.

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

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/test_zswap.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
2.55.0.766.g2966f0265a-goog



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback
  2026-08-24  3:33 [PATCH v2 0/2] selftests/cgroup: fixes for test_zswap on single core VM Wilson Felipe Pereira
@ 2026-08-24  3:33 ` Wilson Felipe Pereira
  2026-08-24 12:36   ` Michal Koutný
  2026-08-24  3:33 ` [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
  1 sibling, 1 reply; 5+ messages in thread
From: Wilson Felipe Pereira @ 2026-08-24  3:33 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Yosry Ahmed, Nhat Pham,
	Chengming Zhou, Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, linux-kernel,
	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>
---
 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 49b36ee79160..a7ff525c1267 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -407,6 +407,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.766.g2966f0265a-goog



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass
  2026-08-24  3:33 [PATCH v2 0/2] selftests/cgroup: fixes for test_zswap on single core VM Wilson Felipe Pereira
  2026-08-24  3:33 ` [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback Wilson Felipe Pereira
@ 2026-08-24  3:33 ` Wilson Felipe Pereira
  2026-08-24 12:37   ` Michal Koutný
  1 sibling, 1 reply; 5+ messages in thread
From: Wilson Felipe Pereira @ 2026-08-24  3:33 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Yosry Ahmed, Nhat Pham,
	Chengming Zhou, Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, linux-kernel,
	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 longs 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>
---
 tools/testing/selftests/cgroup/test_zswap.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index a7ff525c1267..15cc1dfdd1f4 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -621,9 +621,11 @@ 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;
+			long zswapped = cg_read_key_long(
+				test_group, "memory.stat", "zswapped ");
+			long delta = stored_pages * page_size - zswapped;
+			long max_delta = (long)(stored_pages * page_size / 4);
+			int result_ok = delta < max_delta;
 
 			ret = result_ok ? KSFT_PASS : KSFT_FAIL;
 			break;
-- 
2.55.0.766.g2966f0265a-goog



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback
  2026-08-24  3:33 ` [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback Wilson Felipe Pereira
@ 2026-08-24 12:36   ` Michal Koutný
  0 siblings, 0 replies; 5+ messages in thread
From: Michal Koutný @ 2026-08-24 12:36 UTC (permalink / raw)
  To: Wilson Felipe Pereira
  Cc: Andrew Morton, Johannes Weiner, Yosry Ahmed, Nhat Pham,
	Chengming Zhou, Tejun Heo, Shuah Khan, linux-mm, cgroups,
	linux-kselftest, linux-kernel

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

On Mon, Aug 24, 2026 at 03:33:57AM +0000, Wilson Felipe Pereira <wfelipe@google.com> wrote:
> 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>
> ---
>  tools/testing/selftests/cgroup/test_zswap.c | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Michal Koutný <mkoutny@suse.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass
  2026-08-24  3:33 ` [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
@ 2026-08-24 12:37   ` Michal Koutný
  0 siblings, 0 replies; 5+ messages in thread
From: Michal Koutný @ 2026-08-24 12:37 UTC (permalink / raw)
  To: Wilson Felipe Pereira
  Cc: Andrew Morton, Johannes Weiner, Yosry Ahmed, Nhat Pham,
	Chengming Zhou, Tejun Heo, Shuah Khan, linux-mm, cgroups,
	linux-kselftest, linux-kernel

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

On Mon, Aug 24, 2026 at 03:33:58AM +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 longs 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>
> ---
>  tools/testing/selftests/cgroup/test_zswap.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
> index a7ff525c1267..15cc1dfdd1f4 100644
> --- a/tools/testing/selftests/cgroup/test_zswap.c
> +++ b/tools/testing/selftests/cgroup/test_zswap.c
> @@ -621,9 +621,11 @@ 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;
> +			long zswapped = cg_read_key_long(
> +				test_group, "memory.stat", "zswapped ");
> +			long delta = stored_pages * page_size - zswapped;
> +			long max_delta = (long)(stored_pages * page_size / 4);
> +			int result_ok = delta < max_delta;

I'd rewrite it like below for higher type confidence (mainly, an explict
cast of stored_pages so that subtraction is signed; possibly, implicit
cast of expression assinged to max_delta):

			long delta = (long)stored_pages * page_size - zswapped;
			long max_delta = stored_pages * page_size / 4;


>  
>  			ret = result_ok ? KSFT_PASS : KSFT_FAIL;
			ret = (delta < max_delta) ? KSFT_PASS : KSFT_FAIL;

Thanks,
Michal

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-24 12:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24  3:33 [PATCH v2 0/2] selftests/cgroup: fixes for test_zswap on single core VM Wilson Felipe Pereira
2026-08-24  3:33 ` [PATCH v2 1/2] selftests/cgroup: test_zswap: wait for cgroup to unpopulate in test_zswap_writeback Wilson Felipe Pereira
2026-08-24 12:36   ` Michal Koutný
2026-08-24  3:33 ` [PATCH v2 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
2026-08-24 12:37   ` Michal Koutný

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox