Re: [PATCH] selftests/cgroup: ignore memory.reclaim -EAGAIN for zswap writeback test

[Harry Yoo]

On Mon, Sep 14, 2026 at 05:18:17PM -0700, SJ Park wrote:
> Hi Harry,

Hi SJ, thanks for looking into it!

> On Mon, 14 Sep 2026 21:36:39 +0100 "Harry Yoo (Meta)" <harry@xxxxxxxxxx> wrote:
> Makes sense to me.

Thanks!

> [...]
> > --- a/tools/testing/selftests/cgroup/test_zswap.c
> > +++ b/tools/testing/selftests/cgroup/test_zswap.c
> > @@ -346,7 +346,16 @@ static int attempt_writeback(const char *cgroup, void *arg)
> >  	 * it can't writeback to swap.
> >  	 */
> >  	ret = cg_write_numeric(cgroup, "memory.reclaim", memsize);
> > -	if (!wb_enabled)
> > +
> > +	/*
> > +	 * When writeback is enabled, memory.reclaim may still fail to reclaim
> > +	 * the requested amount of memory due to a slow swap device.
> > +	 * Ignore -EAGAIN here. The caller determines pass/fail based on the
> > +	 * zswap writeback counter.
> > +	 */
> > +	if (wb_enabled && ret == -EAGAIN)
> > +		ret = 0;
> > +	else if (!wb_enabled)
> >  		ret = (ret == -EAGAIN) ? 0 : -1;
> 
> My humble eyes were unable to easily understand the change.  Is the change
> effectively same to below, and if so, would this be easier to read?
> 
> '''
> @@ -344,10 +344,11 @@ static int attempt_writeback(const char *cgroup, void *arg)
>          * writeback as zswap.max is 1/4 of what was needed when reclaim ran the first time.
>          * If writeback is disabled, memory reclaim will fail as zswap is limited and
>          * it can't writeback to swap.
> +        * Even if writeback is enabled, it could return -EAGAIN due to a slow
> +        * swap device.
>          */
>         ret = cg_write_numeric(cgroup, "memory.reclaim", memsize);
> -       if (!wb_enabled)
> -               ret = (ret == -EAGAIN) ? 0 : -1;
> +       ret = (ret == -EAGAIN) ? 0 : -1;

Hmm, no. This will mark the test as failed when a write to
memory.reclaim returns zero even when wb_enabled is true.

It's not the same because when wb_enabled == true we consider
ret == 0 or ret == -EAGAIN as success, while with wb_enabled == false
we consider ret == -EAGAIN as success.

Not sure we can reduce that to one line of code.

>  out:
>         free(mem);
> '''

-- 
Cheers,
Harry / Hyeonggon