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)" <[email protected]> > 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
