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

[Hao Jia <jiahao.kernel@gmail.com>]

On 2026/7/16 00:13, Yosry Ahmed wrote:
> On Wed, Jul 15, 2026 at 5:31 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>
>>
>>
>> On 2026/7/15 10:31, Andrew Morton wrote:
>>> On Tue, 14 Jul 2026 09:52:59 -0700 Yosry Ahmed <yosry@kernel.org> 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@vger.kernel.org
>>>>> Suggested-by: Nhat Pham <nphamcs@gmail.com>
>>>>> Acked-by: Nhat Pham <nphamcs@gmail.com>
>>>>> Acked-by: Yosry Ahmed <yosry@kernel.org>
>>>>> Reported-by: Yosry Ahmed <yosry@kernel.org>
>>>>> Closes: https://lore.kernel.org/all/CAO9r8zPVzMKFbCixxD-qgtRrkFxWVrHiZZeLc=eyTPKPVQgX4g@mail.gmail.com
>>>>> Signed-off-by: Hao Jia <jiahao1@lixiang.com>
>>>>
>>>> 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.
Will do. Thanks!

> 
>>
>>> Also, AI review has flagged several possible issues, all appear to be
>>> serious:
>>>        https://sashiko.dev/#/patchset/20260714081510.16895-1-jiahao.kernel@gmail.com
>>
>> 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?

The loop would require the following sequence to repeat indefinitely:

1、zswap_total_pages() > thr evaluates to true.
2、During shrink_memcg(root_memcg), the zswap LRU is concurrently drained 
to empty.
3、Before zswap_total_pages() > thr is evaluated again, the zswap LRU is 
heavily refilled such that zswap_total_pages() > thr holds true once more.

For our case to manifest, it would require zswap_total_pages() and the 
zswap LRU state to repeatedly hit this exact window with perfect 
alignment over a **prolonged period**. Therefore, I suspect this 
scenario might only exist in theory.

Thanks,
Hao