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

From: SJ Park

Date: Tue Sep 15 2026 - 10:25:26 EST


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:
> > 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.

Ah, got it. You are right. Sorry for making noise and thank you for kindly
correcting me.

Reviewed-by: SJ Park <sj@xxxxxxxxxx>


Thanks,
SJ

[...]