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

["Michal Koutný" <mkoutny@suse.com>]

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