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

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

[Hao Jia]

From: Hao Jia <jiahao1@lixiang.com>

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>
---
 mm/zswap.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index b5a17ea20237..3d697a1a5365 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;
 
-- 
2.34.1

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

[Yosry Ahmed]

On Tue, Jul 14, 2026 at 1:15 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
> From: Hao Jia <jiahao1@lixiang.com>
>
> 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?

> ---
>  mm/zswap.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index b5a17ea20237..3d697a1a5365 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;
>
> --
> 2.34.1
>

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

[Andrew Morton]

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.

Also, AI review has flagged several possible issues, all appear to be
serious:
	https://sashiko.dev/#/patchset/20260714081510.16895-1-jiahao.kernel@gmail.com

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@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.

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

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

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@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.

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

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

[Hao Jia]

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

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

[Nhat Pham]

On Wed, Jul 15, 2026 at 7:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> 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.

Yeah seems very artificial indeed.

If we want to be extra careful here, maybe we can put a cond_resched()
there or replace the continue with goto resched or sth?

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

[Yosry Ahmed]

On Thu, Jul 16, 2026 at 11:19 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Wed, Jul 15, 2026 at 7:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
> >
> >
> >
> > 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.
>
> Yeah seems very artificial indeed.
>
> If we want to be extra careful here, maybe we can put a cond_resched()
> there or replace the continue with goto resched or sth?

I think it's also possible with the current code with memcg enabled.
It's still possible that the shrinker puts usage under the acceptance
threshold and then a new zswap stores puts it back above the
threshold, and so on.

Perhaps as Nhat said, just goto resched instead of continue if we're
really worried, but I think even that is not really necessary.

I think we generally want to clean up and simplify the shrinking loop
in zswap_shrinker(), but I don't have any great ideas.

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

[Hao Jia]

On 2026/7/17 08:08, Yosry Ahmed wrote:
> On Thu, Jul 16, 2026 at 11:19 AM Nhat Pham <nphamcs@gmail.com> wrote:
>>
>> On Wed, Jul 15, 2026 at 7:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>>
>>>
>>>
>>> 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.
>>
>> Yeah seems very artificial indeed.
>>
>> If we want to be extra careful here, maybe we can put a cond_resched()
>> there or replace the continue with goto resched or sth?
> 
> I think it's also possible with the current code with memcg enabled.
> It's still possible that the shrinker puts usage under the acceptance
> threshold and then a new zswap stores puts it back above the
> threshold, and so on.
> 
> Perhaps as Nhat said, just goto resched instead of continue if we're
> really worried, but I think even that is not really necessary.
> 

Yes, but to be strictly rigorous, let's add a goto resched; anyway.

Thanks,
Hao

> I think we generally want to clean up and simplify the shrinking loop
> in zswap_shrinker(), but I don't have any great ideas.