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

On Wed, 29 Jul 2026 17:30:56 -0700 Yosry Ahmed <[email protected]> wrote:

> On Wed, Jul 29, 2026 at 3:58 PM Andrew Morton <[email protected]> wrote:
> >
> > On Wed, 29 Jul 2026 16:42:05 +0800 Hao Jia <[email protected]> wrote:
> >
> > > Zswap writeback when the global pool limit is hit fails when memory
> > > cgroup is disabled. The pool remains full until it is organically
> > > drained by swapins or memory freeing, leading to zswap store failures
> > > and pages bypassing getting written directly to the backing swap device,
> > > causing LRU inversion (hotter pages with higher fault latency).
> > >
> > > This happens because mem_cgroup_iter() always returns NULL when
> > > memory cgroups are disabled. As a result, the global shrinker
> > > shrink_worker() repeatedly takes empty walks. After MAX_RECLAIM_RETRIES
> > > failed attempts, the worker gives up without writing back any pages.
> > >
> > > 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. In the
> > > absence of heavy concurrent zswap stores, the loop then safely bails out
> > > via the zswap_total_pages() <= thr check; otherwise, it will resume
> > > shrinking the memcg after processing the reschedule 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: [email protected]
> >
> > How does this affect users?  What behavior do they observe when it
> > occurs?
> 
> I think the first paragraph sums it up pretty well, especially the
> last sentence "hotter pages with higher fault latency".

How do users observe that?

See, what I'm looking for here is an explanation for why we're
proposing a backport.  How are our users harmed by the current code and
how does this change benefit them?

> >
> > > Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
> >
> > hm, that isn't really a bug report and doesn't answer the above
> > question.
> 
> Yeah, it isn't. Probably we should drop "Closes". I assume Hao added
> it because checkpatch annoyingly complains if you add "Reported-by"
> without "Closes", so Hao just linked to the thread where I pointed out
> the bug.

OK.