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

[Yosry Ahmed]

On Wed, Jul 15, 2026 at 5:31 AM Hao Jia <jiahao.kernel@xxxxxxxxx> wrote:
>
>
>
> On 2026/7/15 10:31, Andrew Morton wrote:
> > On Tue, 14 Jul 2026 09:52:59 -0700 Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
> >
> >>> 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.
> >>>
> >>> With memcg disabled, shrink_memcg() only returns -ENOENT when the root
> >>> LRU is empty, which means the total pages are already below thr. The
> >>> loop then safely bails out via the zswap_total_pages() <= thr check.
> >>> For any other return value from shrink_memcg(), the loop is guaranteed
> >>> to terminate, either after MAX_RECLAIM_RETRIES failures or once the
> >>> threshold is met.
> >>>
> >>> Fixes: a65b0e7607cc ("zswap: make shrinking memcg-aware")
> >>> Cc: stable@xxxxxxxxxxxxxxx
> >>> Suggested-by: Nhat Pham <nphamcs@xxxxxxxxx>
> >>> Acked-by: Nhat Pham <nphamcs@xxxxxxxxx>
> >>> Acked-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> >>> Reported-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> >>> Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@xxxxxxxxxxxxxx
> >>> Signed-off-by: Hao Jia <jiahao1@xxxxxxxxxxx>
> >>
> >> Patch 2 doesn't really depend on this one, right?
> >>
> >> If that's the case I think this can (and should be) picked up
> >> separately as a hotfix. Andrew, WDYT?
> >
> > Please update the changelog to clearly describe the userspace-visible
> > effects of the bug, thanks.
>
> I am not entirely sure if my understanding is correct here, but maybe I
> should add something like this to the commit message?
>
> When cgroup_disable=memory is used (or with CONFIG_MEMCG=n), the global
> shrinker fails to write back any pages. Consequently, the zswap pool
> fills up to its limit and rejects further storage, preventing memory
> pressure from being offloaded to the backing swap device.

I think you can simply write that zswap writeback when the limit is
hit is broken when memcg is disabled.

>
> > Also, AI review has flagged several possible issues, all appear to be
> > serious:
> >       https://sashiko.dev/#/patchset/20260714081510.16895-1-jiahao.kernel@xxxxxxxxx
>
> For AI review comments on this patch:
> I suspect this scenario might only exist in theory. For zswap LRU to be
> empty while zswap_total_pages() > thr holds true, it would require a
> prolonged state where there are always more than thr zswap entries on
> the zswap LRU whenever zswap_total_pages() > thr is evaluated, yet the
> zswap LRU happens to be empty during shrink_memcg(root_memcg).
>
> If we want to fix this, perhaps we could do something like this?
>
> Yosry, Nhat, what are your thoughts on this?

Do we need to do this? The last paragraph in your changelog explains
why this can't happen because zswap_total_pages() should be 0 in this
case. Did I miss something?