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

On Mon, Aug 3, 2026 at 9:21=E2=80=AFPM Wilson Felipe Pereira <wfelipe@googl=
e.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 create=
d
> by test_zswap_writeback_one(), writing "+memory" to cgroup.subtree_contro=
l
> can fail with -EBUSY. Under Cgroup v2, enabling domain subtree controller=
s
> 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 <[email protected]>
> ---
>  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, bo=
ol 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 !=3D 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 =3D cg_name(test_group, "zswap_writeback_test_ch=
ild");
>         if (!test_group_child)
> --
> 2.55.0.571.g244d577d93-goog
>