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

[Hao Jia]

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.

> 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?

diff --git a/mm/zswap.c b/mm/zswap.c
index b5a17ea20237..ca71b517a58d 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1356,11 +1356,12 @@ static void shrink_worker(struct work_struct *w)
                } while (memcg && !mem_cgroup_tryget_online(memcg));
                spin_unlock(&zswap_shrink_lock);

-               if (!memcg) {
-                       /*
-                        * Continue shrinking without incrementing 
failures if
-                        * we found candidate memcgs in the last tree walk.
-                        */
+               /*
+                * A NULL memcg ends a full hierarchy pass (except when 
memcg is
+                * disabled, where it is always NULL: fall through to 
the root LRU).
+                * Count a failure only if the last pass found no 
candidates.
+                */
+               if (!memcg && !mem_cgroup_disabled()) {
                        if (!attempts && ++failures == MAX_RECLAIM_RETRIES)
                                break;

@@ -1378,8 +1379,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;
                        continue;
+               }
                ++attempts;

                if (ret && ++failures == MAX_RECLAIM_RETRIES)


Thanks,
Hao