Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes
@ 2026-08-04  4:20 Wilson Felipe Pereira
  2026-08-04  4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
  2026-08-04  4:20 ` [PATCH 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-04  4:20 UTC (permalink / raw)
  To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou,
	Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, Wilson Felipe Pereira

When running the cgroup zswap selftests on a single-core VM (-smp 1) with
4GB of RAM, test_zswap_writeback and test_no_kmem_bypass reliably fail on
the initial run after boot due to timing and asynchronous tasks not
completed.

This 2-patch series fixes both initial-run failures:

  * Patch 1 fixes an EBUSY race in test_zswap_writeback() where writing
    "+memory" to cgroup.subtree_control collides with the kernel's
    asynchronous RCU removal of dead child tasks from cgroup.procs.

  * Patch 2 fixes an implicit unsigned integer promotion bug in
    test_no_kmem_bypass() where a small negative delta becomes large when
    compared to an unsigned size_t threshold.

Both tests now reliably pass on the initial run after cold boot on -smp 1
VMs.


Wilson Felipe Pereira (2):
  selftests/cgroup: test_zswap: retry subtree_control write on EBUSY 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.571.g244d577d93-goog


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

* [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback
  2026-08-04  4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira
@ 2026-08-04  4:20 ` Wilson Felipe Pereira
       [not found]   ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>
  2026-08-04  4:20 ` [PATCH 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-04  4:20 UTC (permalink / raw)
  To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou,
	Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, Wilson Felipe Pereira

When running test_zswap on a single-core VM (-smp 1) with 4GB of RAM,
test_zswap_writeback reliably 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, waitpid() reaps the zombie PID immediately,
but the removal of struct task_struct from the cgroup task list is
performed asynchronously via an RCU callback (release_task). On
single-core systems, this RCU callback is delayed behind CPU softirqs,
causing "+memory" to fail if written immediately after waitpid() returns.

Fix this by adding an EBUSY retry loop with usleep(1000) around cg_write(),
matching the existing cgroup cleanup pattern in cg_destroy().

Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com>
---
 tools/testing/selftests/cgroup/test_zswap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index 49b36ee791606..312d51887a1d8 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -407,8 +407,11 @@ 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_write(test_group, "cgroup.subtree_control", "+memory"))
-		goto out;
+	while (cg_write(test_group, "cgroup.subtree_control", "+memory")) {
+		if (errno != EBUSY)
+			goto out;
+		usleep(1000);
+	}
 
 	test_group_child = cg_name(test_group, "zswap_writeback_test_child");
 	if (!test_group_child)
-- 
2.55.0.571.g244d577d93-goog



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

* [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass
  2026-08-04  4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira
  2026-08-04  4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
@ 2026-08-04  4:20 ` Wilson Felipe Pereira
  2026-08-06  8:32   ` Michal Koutný
  1 sibling, 1 reply; 5+ messages in thread
From: Wilson Felipe Pereira @ 2026-08-04  4:20 UTC (permalink / raw)
  To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou,
	Tejun Heo, Michal Koutný, Shuah Khan
  Cc: linux-mm, cgroups, linux-kselftest, 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 checking abs(delta), ensuring the test correctly compares the
absolute difference between system zswap and cgroup zswapped bytes.

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

diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
index 312d51887a1d8..7bc2519862f7e 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -624,7 +624,8 @@ static int test_no_kmem_bypass(const char *root)
 		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;
+			int result_ok = abs(delta) <
+					stored_pages * page_size / 4;
 
 			ret = result_ok ? KSFT_PASS : KSFT_FAIL;
 			break;
-- 
2.55.0.571.g244d577d93-goog



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

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

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

On Tue, Aug 04, 2026 at 04:20:45AM +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.

Thanks for the breakdown.

> Fix this by checking abs(delta), ensuring the test correctly compares the
> absolute difference between system zswap and cgroup zswapped bytes.

I still think the delta quantity has some meaning here and taking the
abs() changes the semantics of the checked inequality.

What about making both zswapped and delta `long`s? (Under similar
reasoning, there's already a possible loss after squashing
cg_read_key_long() result into the `int`.)

In any way, also
Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test")

Thanks,
Michal

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

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

* Re: [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback
       [not found]   ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>
@ 2026-08-06  8:33     ` Michal Koutný
  0 siblings, 0 replies; 5+ messages in thread
From: Michal Koutný @ 2026-08-06  8:33 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Wilson Felipe Pereira, Johannes Weiner, Nhat Pham, Chengming Zhou,
	Tejun Heo, Shuah Khan, linux-mm, cgroups, linux-kselftest

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

On Mon, Aug 03, 2026 at 11:39:14PM -0700, Yosry Ahmed <yosry@kernel.org> wrote:
> > diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c
> > index 49b36ee791606..312d51887a1d8 100644
> > --- a/tools/testing/selftests/cgroup/test_zswap.c
> > +++ b/tools/testing/selftests/cgroup/test_zswap.c
> > @@ -407,8 +407,11 @@ 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_write(test_group, "cgroup.subtree_control", "+memory"))
> > -               goto out;
> > +       while (cg_write(test_group, "cgroup.subtree_control", "+memory")) {
> > +               if (errno != EBUSY)
> > +                       goto out;
> > +               usleep(1000);
> > +       }
> 
> I would honestly rather create a new cgroup here instead of reusing
> the leaf cgroup as a parent, to avoid any subtleties like this in the
> future, or even split test_zswap_writeback() into two test cases. I
> don't feel strongly though if others think the simple wait here is
> enough.

I somewhat agree, I'm not sure whether the recycling brings any signal
to the test.

In the case, the recycling and (re-)enablement is needed, I'd suggest an
explicit check for emptiness, the best of utils arsenal seems:
	cg_read_strcmp_wait(test_cgroup, "cgroup.events", "populated 0\n")

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-06  8:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira
2026-08-04  4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
     [not found]   ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>
2026-08-06  8:33     ` Michal Koutný
2026-08-04  4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
2026-08-06  8:32   ` Michal Koutný

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