On Tue, 15 Sep 2026 11:21:53 +0100 Harry Yoo <[email protected]> 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)" <[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. Ah, got it. You are right. Sorry for making noise and thank you for kindly correcting me. Reviewed-by: SJ Park <[email protected]> Thanks, SJ [...]
