On Tue, Sep 15, 2026 at 07:10:48AM -0700, SJ Park wrote: > 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: > > > On Mon, 14 Sep 2026 21:36:39 +0100 "Harry Yoo (Meta)" <[email protected]> > > > 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 <[email protected]> Thanks a lot for reviewing, SJ! -- Cheers, Harry / Hyeonggon
