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

[Harry Yoo]

On Tue, Sep 15, 2026 at 07:10:48AM -0700, SJ Park wrote:
> On Tue, 15 Sep 2026 11:21:53 +0100 Harry Yoo <harry@xxxxxxxxxx> wrote:
> 
> > On Mon, Sep 14, 2026 at 05:18:17PM -0700, SJ Park wrote:
> > > On Mon, 14 Sep 2026 21:36:39 +0100 "Harry Yoo (Meta)" <harry@xxxxxxxxxx> wrote:
> > > > --- 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.
> 
> Ah, got it.  You are right.  Sorry for making noise and thank you for kindly
> correcting me.

No worries ;-)
 
> Reviewed-by: SJ Park <sj@xxxxxxxxxx>

Thanks a lot for reviewing, SJ!

-- 
Cheers,
Harry / Hyeonggon