[PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes

[PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes

[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

[PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY 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 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

[PATCH 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 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

Re: [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback

[Yosry Ahmed]

On Mon, Aug 3, 2026 at 9:21 PM 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 reliably fails on the initial run after boot.

I see why this is related to having a single core, but I am not sure
why it would only happen for the first 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.

IIUC, the rejection to update subtree_control comes from
cgroup_vet_subtree_control_enable() -> cgroup_has_tasks(), which reads
nr_populated_csets.

This seems to be updated in this path when a task exits:
finish_task_switch()
cgroup_task_dead()
do_cgroup_task_dead() (in !CONFIG_PREEMPT_RT)
css_set_move_task()
css_set_update_populated()
css_update_populated()

So outside of CONFIG_PREEMPT_RT, I don't think the update is deferred.

Probably what's actually happening is that parent gets woken up
through do_exit() -> exit_notify(), then the task is marked as
TASK_DEAD in do_task_dead(), and finally finish_task_switch() calls
cgroup_task_dead()?

>
> 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);
> +       }

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.

>
>         test_group_child = cg_name(test_group, "zswap_writeback_test_child");
>         if (!test_group_child)
> --
> 2.55.0.571.g244d577d93-goog
>

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

[Michal Koutný]

[-- 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 --]

Re: [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback

[Michal Koutný]

[-- 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 --]