Re: [PATCH v5 1/6] mm/zswap: Fix global shrinker when memory cgroup is disabled

[Nhat Pham]

On Mon, Jun 29, 2026 at 4:20 AM Hao Jia <jiahao.kernel@xxxxxxxxx> wrote:
>
> From: Hao Jia <jiahao1@xxxxxxxxxxx>
>
> When memory cgroup is disabled, mem_cgroup_iter() always returns NULL.
> Therefore, the global shrinker shrink_worker() always takes the !memcg
> branch. After MAX_RECLAIM_RETRIES empty walks, the worker simply gives up,
> so it fails to write back anything.
>
> Therefore, when memory cgroup is disabled, fall through with the !memcg
> branch and shrink the root memcg directly. Stop the loop once
> shrink_memcg() reports -ENOENT, since the root LRU is the only target and
> -ENOENT means it has been exhausted.
>
> Fixes: a65b0e7607cc ("zswap: make shrinking memcg-aware")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@xxxxxxxxxxxxxx
> Signed-off-by: Hao Jia <jiahao1@xxxxxxxxxxx>

Ah good catch.



> ---
>  mm/zswap.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 761cd699e0a3..0f8f04f22888 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1356,7 +1356,12 @@ static void shrink_worker(struct work_struct *w)
>                 } while (memcg && !mem_cgroup_tryget_online(memcg));
>                 spin_unlock(&zswap_shrink_lock);
>
> -               if (!memcg) {
> +               /*
> +                * Reaching a NULL memcg means a full hierarchy pass completed.
> +                * Exclude the memcg-disabled case, where it is always NULL, and
> +                * fall through to shrink the root LRU directly.
> +                */
> +               if (!memcg && !mem_cgroup_disabled()) {
>                         /*
>                          * Continue shrinking without incrementing failures if
>                          * we found candidate memcgs in the last tree walk.

nit: I wonder if we can just merge this comment with the new comment
you just added.

> @@ -1378,8 +1383,15 @@ static void shrink_worker(struct work_struct *w)
>                  * with pages in zswap. Skip this without incrementing attempts
>                  * and failures.
>                  */
> -               if (ret == -ENOENT)
> +               if (ret == -ENOENT) {
> +                       /*
> +                        * With memcg disabled the root LRU is the only target, so
> +                        * we should abort if it has no writeback-candidate pages.
> +                        */
> +                       if (mem_cgroup_disabled())
> +                               break;

Hmm do we need to do this? Consider a system with cgroup enabled but
with just one cgroup (root?). The behavior would just be trying that
cgroup for MAX_RECLAIM_RETRIES failure attempts, correct?

In that case, we don't need to do this check, and we would get the
same behavior. The loop would terminate after MAX_RECLAIM_RETRIES :)

Could you fact-check me? :)