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

[SJ Park <sj@kernel.org>]

Hi Harry,

On Mon, 14 Sep 2026 21:36:39 +0100 "Harry Yoo (Meta)" <harry@kernel.org> wrote:

> The zswap_writeback_enabled test fails when a write to memory.reclaim
> returns -EAGAIN, which means less than the requested amount was
> reclaimed. attempt_writeback() propagates the -EAGAIN to the caller,
> and the test case is marked as failed even when zswap writeback did
> happen.
> 
> This heavily depends on the performance of the backing swap device.
> Reclaim does not wait for writeback (on cgroup v2), does not count pages
> that are under writeback as reclaimed, and memory.reclaim gives up after
> MAX_RECLAIM_RETRIES passes without making progress. On a slow device
> where reclaim does not make any progress before writeback completes,
> a write to memory.reclaim fails.
> 
> On a VM with zswap enabled, where IO delay was injected via dm-delay,
> the success rate of the zswap writeback test drops dramatically once
> the delay reaches 11 ms: 7% failures at 10 ms and 79% failures at 11 ms,
> n = 100.
> 
> When zswap writeback is enabled, ignore -EAGAIN from memory.reclaim and
> determine pass/fail based on the zswpwb counter because that is what
> zswap_writeback_enabled actually wants to test.
> 
> With this change, the test reliably passes even on a slow swap device
> (tested up to 1000 ms delay). This makes the test resilient against
> the performance of the swap device.

Makes sense to me.

[...]
> --- 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;

 out:
        free(mem);
'''

Someone might hate the second 'ret' assignment.  But I was unable to make it
look cleaner without introducing a >80 column line.  The file already has
multiple >80 column lines and I don't really mind having a new long line,
though.


Thanks,
SJ

[...]