Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Yosry Ahmed]

On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> From: Hao Jia <jiahao1@lixiang.com>
> 
> Currently, shrink_memcg() writes back at most one entry per-node during
> its traversal. This makes shrink_worker() inefficient, as it must
> repeatedly re-enter shrink_memcg() to make any substantial progress.
> Under high memory pressure, this can cause the writeback speed to be
> too slow to keep up with refaults, leading to zswap store failures and
> forcing pages to skip zswap and go directly to disk, which results in
> an LRU inversion.
> 
> To address this, extend shrink_memcg() and rewrite its LRU iteration logic
> to support batch writeback. Introduce the nr_to_scan parameter to bound how
> many pages are scanned per call. This enables batch writeback in the
> shrink_worker() path, while maintaining a low scan budget in the
> zswap_store() path.
> 
> Test Setup:
> - Total memory: 32 GB.
> - zswap settings: max_pool_percent=1, accept_threshold_percent=50,
>   shrinker_enabled=N.
> 
> Test Case 1:
> Allocate 512MB of anonymous pages and fill them with random data (to avoid
> compression), then use cgroup memory.reclaim to force a large amount of
> anonymous pages into zswap. At an interval of 2ms, allocate a 4K anonymous
> page where the first 4 bytes are random numbers and the rest are zeros, and
> then trigger a reclamation of this 4K anonymous page through cgroup
> memory.reclaim. When the pool threshold is reached, shrink_memcg() will
> be triggered.
> The test data after running for 120s is as follows:
>                                 Baseline       Patched
> shrink_worker wakeups              5,363            85
> shrink_memcg calls            11,373,201       180,928
> written_back pages                40,212        40,236
> zswap_store calls                161,190       168,741
>    store succeeded (ret=1)       102,743       127,644
>    store rejected (ret=0)         58,447        41,097
>    store reject rate                ~36%          ~24%
> pool_limit_hit delta              55,826        14,062
> pswpout                           98,659        81,333
> pswpin                                 2             1
> 
> Test Case 2:
> To consistently force zswap store failures and trigger shrink_worker(),
> the following stress-ng command was run for 120 seconds within a cgroup
> limited to a memory.max of 1G:
>    bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \
>    exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t \
> 120s -q'
> The test data after running for 120s is as follows:
>                                 Baseline       Patched
> shrink_worker wakeups              5,640           987
> shrink_memcg calls             8,481,500     2,504,818
> written_back pages                   260       768,576
> zswap_store calls              2,742,756     2,301,414
>    store succeeded (ret=1)       934,640     1,308,686
>    store rejected (ret=0)      1,808,116       992,728
>    store reject rate                ~66%          ~43%
> pool_limit_hit delta           1,181,310       101,593
> pswpout                        1,808,376     1,761,304
> pswpin                         4,288,497     3,902,658
> 
> Under identical workloads and runtimes, batching the zswap shrinker
> exhibits a significant reduction in both shrink_worker wakeups and
> shrink_memcg calls. Furthermore, the sharp drop in both pool_limit_hit
> and zswap_store rejections demonstrates that batching the zswap shrinker
> effectively mitigates zswap_store failures caused by hitting the pool
> limit. This significantly prevents pages from bypassing zswap and falling
> back directly to disk, thereby reducing LRU inversion.
> 
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Hao Jia <jiahao1@lixiang.com>

Acked-by: Yosry Ahmed <yosry@kernel.org>

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Nhat Pham]

On Fri, Jul 17, 2026 at 1:52 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
> From: Hao Jia <jiahao1@lixiang.com>
>
> Currently, shrink_memcg() writes back at most one entry per-node during
> its traversal. This makes shrink_worker() inefficient, as it must
> repeatedly re-enter shrink_memcg() to make any substantial progress.
> Under high memory pressure, this can cause the writeback speed to be
> too slow to keep up with refaults, leading to zswap store failures and
> forcing pages to skip zswap and go directly to disk, which results in
> an LRU inversion.
>
> To address this, extend shrink_memcg() and rewrite its LRU iteration logic
> to support batch writeback. Introduce the nr_to_scan parameter to bound how
> many pages are scanned per call. This enables batch writeback in the
> shrink_worker() path, while maintaining a low scan budget in the
> zswap_store() path.
>
> Test Setup:
> - Total memory: 32 GB.
> - zswap settings: max_pool_percent=1, accept_threshold_percent=50,
>   shrinker_enabled=N.
>
> Test Case 1:
> Allocate 512MB of anonymous pages and fill them with random data (to avoid
> compression), then use cgroup memory.reclaim to force a large amount of
> anonymous pages into zswap. At an interval of 2ms, allocate a 4K anonymous
> page where the first 4 bytes are random numbers and the rest are zeros, and
> then trigger a reclamation of this 4K anonymous page through cgroup
> memory.reclaim. When the pool threshold is reached, shrink_memcg() will
> be triggered.
> The test data after running for 120s is as follows:
>                                 Baseline       Patched
> shrink_worker wakeups              5,363            85
> shrink_memcg calls            11,373,201       180,928
> written_back pages                40,212        40,236
> zswap_store calls                161,190       168,741
>    store succeeded (ret=1)       102,743       127,644
>    store rejected (ret=0)         58,447        41,097
>    store reject rate                ~36%          ~24%
> pool_limit_hit delta              55,826        14,062
> pswpout                           98,659        81,333
> pswpin                                 2             1
>
> Test Case 2:
> To consistently force zswap store failures and trigger shrink_worker(),
> the following stress-ng command was run for 120 seconds within a cgroup
> limited to a memory.max of 1G:
>    bash -c 'echo $$ > /sys/fs/cgroup/zswaptest/cgroup.procs ; \
>    exec stress-ng --vm 4 --vm-bytes 4G --vm-keep --vm-method rand-set -t \
> 120s -q'
> The test data after running for 120s is as follows:
>                                 Baseline       Patched
> shrink_worker wakeups              5,640           987
> shrink_memcg calls             8,481,500     2,504,818
> written_back pages                   260       768,576
> zswap_store calls              2,742,756     2,301,414
>    store succeeded (ret=1)       934,640     1,308,686
>    store rejected (ret=0)      1,808,116       992,728
>    store reject rate                ~66%          ~43%
> pool_limit_hit delta           1,181,310       101,593
> pswpout                        1,808,376     1,761,304
> pswpin                         4,288,497     3,902,658
>

Acked-by: Nhat Pham <nphamcs@gmail.com>

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Andrew Morton]

On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> wrote:

> This series fixes and improves the zswap global shrinker (shrink_worker()):
> Patch 1: Fix missing global shrinker when memory cgroup is disabled.
> Patch 2: Extend shrink_memcg() to support batch writeback and update its
>          return value semantics, thereby improving the writeback efficiency
>          in the shrink_worker() path.

Thanks.

[1/2] is a cc:stable fix so it isn't really appropriate to combine this
with [2/2] which doesn't fix any bugs.  Because the two patches may
take different paths into mainline, with different timings.  But that's
OK, I can deal with the splitup if needed.

The [1/2] changelog lacks a description of how the flaw impacts users. 
Please describe this fully and maintain that info within the
changelogging.  This info helps -stable maintainers and others
understand why we're proposing a backport and helps myself and others
with timing decisions.

Finally, AI review might have found an issue:
	https://sashiko.dev/#/patchset/20260717085151.22822-1-jiahao.kernel@gmail.com

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Yosry Ahmed]

On Fri, Jul 17, 2026 at 6:18 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> wrote:
>
> > This series fixes and improves the zswap global shrinker (shrink_worker()):
> > Patch 1: Fix missing global shrinker when memory cgroup is disabled.
> > Patch 2: Extend shrink_memcg() to support batch writeback and update its
> >          return value semantics, thereby improving the writeback efficiency
> >          in the shrink_worker() path.
>
> Thanks.
>
> [1/2] is a cc:stable fix so it isn't really appropriate to combine this
> with [2/2] which doesn't fix any bugs.  Because the two patches may
> take different paths into mainline, with different timings.  But that's
> OK, I can deal with the splitup if needed.
>
> The [1/2] changelog lacks a description of how the flaw impacts users.
> Please describe this fully and maintain that info within the
> changelogging.  This info helps -stable maintainers and others
> understand why we're proposing a backport and helps myself and others
> with timing decisions.
>
> Finally, AI review might have found an issue:
>         https://sashiko.dev/#/patchset/20260717085151.22822-1-jiahao.kernel@gmail.com

We discussed this one in the previous version, it's a theoretical
scenario that can already happen today.

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Yosry Ahmed]

On Fri, Jul 17, 2026 at 6:18 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 17 Jul 2026 16:51:49 +0800 Hao Jia <jiahao.kernel@gmail.com> wrote:
>
> > This series fixes and improves the zswap global shrinker (shrink_worker()):
> > Patch 1: Fix missing global shrinker when memory cgroup is disabled.
> > Patch 2: Extend shrink_memcg() to support batch writeback and update its
> >          return value semantics, thereby improving the writeback efficiency
> >          in the shrink_worker() path.
>
> Thanks.
>
> [1/2] is a cc:stable fix so it isn't really appropriate to combine this
> with [2/2] which doesn't fix any bugs.  Because the two patches may
> take different paths into mainline, with different timings.  But that's
> OK, I can deal with the splitup if needed.

Thank you!

>
> The [1/2] changelog lacks a description of how the flaw impacts users.
> Please describe this fully and maintain that info within the
> changelogging.  This info helps -stable maintainers and others
> understand why we're proposing a backport and helps myself and others
> with timing decisions.

The first line in the changelog should be sufficient imo: "Zswap
writeback on hitting the pool limit is broken when memory cgroup is
disabled"

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Andrew Morton]

On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:

> >
> > The [1/2] changelog lacks a description of how the flaw impacts users.
> > Please describe this fully and maintain that info within the
> > changelogging.  This info helps -stable maintainers and others
> > understand why we're proposing a backport and helps myself and others
> > with timing decisions.
> 
> The first line in the changelog should be sufficient imo: "Zswap
> writeback on hitting the pool limit is broken when memory cgroup is
> disabled"

"broken"?  Perhaps this means "fails to occur".

But what is the userspace-visible impact?  IOW, why are we proposing a
backport?

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Hao Jia]

On 2026/7/18 12:40, Andrew Morton wrote:
> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
> 
>>>
>>> The [1/2] changelog lacks a description of how the flaw impacts users.
>>> Please describe this fully and maintain that info within the
>>> changelogging.  This info helps -stable maintainers and others
>>> understand why we're proposing a backport and helps myself and others
>>> with timing decisions.
>>
>> The first line in the changelog should be sufficient imo: "Zswap
>> writeback on hitting the pool limit is broken when memory cgroup is
>> disabled"
> 
> "broken"?  Perhaps this means "fails to occur".
> 
> But what is the userspace-visible impact?  IOW, why are we proposing a
> backport?
> 

Perhaps the first paragraph of the commit1 message could be modified as 
follows? I have added a description of the issues that occur without 
this patch.

Zswap writeback on hitting the pool limit fails to occur when memory 
cgroup is disabled, because 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. As a result, once the pool 
reaches the zswap limit, every subsequent zswap shrink work run is a 
no-op. This leads to zswap store failures, forcing pages to bypass zswap 
and be written directly to the backing swap device, which can trigger 
issues such as LRU inversion.


Thanks,
Hao

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Hao Jia]

On 2026/7/20 09:26, Hao Jia wrote:
> 
> 
> On 2026/7/18 12:40, Andrew Morton wrote:
>> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
>>
>>>>
>>>> The [1/2] changelog lacks a description of how the flaw impacts users.
>>>> Please describe this fully and maintain that info within the
>>>> changelogging.  This info helps -stable maintainers and others
>>>> understand why we're proposing a backport and helps myself and others
>>>> with timing decisions.
>>>
>>> The first line in the changelog should be sufficient imo: "Zswap
>>> writeback on hitting the pool limit is broken when memory cgroup is
>>> disabled"
>>
>> "broken"?  Perhaps this means "fails to occur".
>>
>> But what is the userspace-visible impact?  IOW, why are we proposing a
>> backport?
>>
> 
> Perhaps the first paragraph of the commit1 message could be modified as 
> follows? I have added a description of the issues that occur without 
> this patch.
> 

Hi Andrew, Yosry, and Nhat,
Any thoughts on this change?

Thanks,
Hao
> Zswap writeback on hitting the pool limit fails to occur when memory 
> cgroup is disabled, because 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. As a result, once the pool 
> reaches the zswap limit, every subsequent zswap shrink work run is a 
> no-op. This leads to zswap store failures, forcing pages to bypass zswap 
> and be written directly to the backing swap device, which can trigger 
> issues such as LRU inversion.
> 
> 
> Thanks,
> Hao

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

[Johannes Weiner]

On Fri, Jul 17, 2026 at 04:51:50PM +0800, Hao Jia wrote:
> From: Hao Jia <jiahao1@lixiang.com>
> 
> Zswap writeback on hitting the pool limit is broken when memory cgroup
> is disabled, because 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. 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: 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>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Johannes Weiner]

On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> @@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
>  			goto resched;
>  		}
>  
> -		ret = shrink_memcg(memcg);
> +		ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
>  		/* drop the extra reference */
>  		mem_cgroup_put(memcg);
>  
> @@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
>  	objcg = get_obj_cgroup_from_folio(folio);
>  	if (objcg && !obj_cgroup_may_zswap(objcg)) {
>  		memcg = get_mem_cgroup_from_objcg(objcg);
> -		if (shrink_memcg(memcg)) {
> +		if (shrink_memcg(memcg, 1)) {

Why 64 for the global limit but only 1 for the cgroup limit? That
seems arbitrary in multiple ways.

Direct reclaim, kswapd, proactive reclaim, cgroup limit reclaim use
SWAP_CLUSTER_MAX for the batch size near-universally. It's magic too
to be sure, but at least you wouldn't have to make up new magic?

Otherwise, the patch looks good to me.

Re: [PATCH v2 0/2] mm/zswap: Fixes and improves the zswap global shrinker

[Yosry Ahmed]

On Wed, Jul 22, 2026 at 6:21 PM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/7/20 09:26, Hao Jia wrote:
> >
> >
> > On 2026/7/18 12:40, Andrew Morton wrote:
> >> On Fri, 17 Jul 2026 18:28:04 -0700 Yosry Ahmed <yosry@kernel.org> wrote:
> >>
> >>>>
> >>>> The [1/2] changelog lacks a description of how the flaw impacts users.
> >>>> Please describe this fully and maintain that info within the
> >>>> changelogging.  This info helps -stable maintainers and others
> >>>> understand why we're proposing a backport and helps myself and others
> >>>> with timing decisions.
> >>>
> >>> The first line in the changelog should be sufficient imo: "Zswap
> >>> writeback on hitting the pool limit is broken when memory cgroup is
> >>> disabled"
> >>
> >> "broken"?  Perhaps this means "fails to occur".
> >>
> >> But what is the userspace-visible impact?  IOW, why are we proposing a
> >> backport?
> >>
> >
> > Perhaps the first paragraph of the commit1 message could be modified as
> > follows? I have added a description of the issues that occur without
> > this patch.
> >
>
> Hi Andrew, Yosry, and Nhat,
> Any thoughts on this change?

I would front load the user impact in the first paragraph:

Zswap writeback when the global pool limit is hit fails when memory
cgroups are 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).

...

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Yosry Ahmed]

On Wed, Jul 22, 2026 at 7:27 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> > @@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
> >                       goto resched;
> >               }
> >
> > -             ret = shrink_memcg(memcg);
> > +             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
> >               /* drop the extra reference */
> >               mem_cgroup_put(memcg);
> >
> > @@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
> >       objcg = get_obj_cgroup_from_folio(folio);
> >       if (objcg && !obj_cgroup_may_zswap(objcg)) {
> >               memcg = get_mem_cgroup_from_objcg(objcg);
> > -             if (shrink_memcg(memcg)) {
> > +             if (shrink_memcg(memcg, 1)) {
>
> Why 64 for the global limit but only 1 for the cgroup limit? That
> seems arbitrary in multiple ways.

I suggested that we keep the writeback here without batching and do
that change separately, mainly out of abundance of caution as
writeback is done synchronously here so the extra latency could be
problematic. I think we probably want to measure the performance
impact of that separately.

That being said, this path is potentially too expensive anyway due to
the flush, but I would rather we do some basic measurements before
batching here.

What do you think?

> Direct reclaim, kswapd, proactive reclaim, cgroup limit reclaim use
> SWAP_CLUSTER_MAX for the batch size near-universally. It's magic too
> to be sure, but at least you wouldn't have to make up new magic?
>
> Otherwise, the patch looks good to me.

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Johannes Weiner]

On Wed, Jul 22, 2026 at 09:52:18PM -0700, Yosry Ahmed wrote:
> On Wed, Jul 22, 2026 at 7:27 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> > > @@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
> > >                       goto resched;
> > >               }
> > >
> > > -             ret = shrink_memcg(memcg);
> > > +             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
> > >               /* drop the extra reference */
> > >               mem_cgroup_put(memcg);
> > >
> > > @@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
> > >       objcg = get_obj_cgroup_from_folio(folio);
> > >       if (objcg && !obj_cgroup_may_zswap(objcg)) {
> > >               memcg = get_mem_cgroup_from_objcg(objcg);
> > > -             if (shrink_memcg(memcg)) {
> > > +             if (shrink_memcg(memcg, 1)) {
> >
> > Why 64 for the global limit but only 1 for the cgroup limit? That
> > seems arbitrary in multiple ways.
> 
> I suggested that we keep the writeback here without batching and do
> that change separately, mainly out of abundance of caution as
> writeback is done synchronously here so the extra latency could be
> problematic. I think we probably want to measure the performance
> impact of that separately.
> 
> That being said, this path is potentially too expensive anyway due to
> the flush, but I would rather we do some basic measurements before
> batching here.
> 
> What do you think?

It's not an unknown, right? We know this works for direct reclaimers,
cgroup limit reclaim e.g., and what the latency implications are.

Because of how reclaim works, we also know it'll call zswap_store() in
batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to
each call shrink_memcg() once we're at the limit - while still risking
rejections due to compressibility differences.

My worry is that if we start with an inconsistency, we'll be stuck
with it for a long time.

I'd rather start with the clean, consistent version. Dial it back only
if we have data to justfiy the complication that we can put into a
comment and the changelog that outlines why exactly it's different.

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Yosry Ahmed]

On Thu, Jul 23, 2026 at 6:55 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Wed, Jul 22, 2026 at 09:52:18PM -0700, Yosry Ahmed wrote:
> > On Wed, Jul 22, 2026 at 7:27 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > >
> > > On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> > > > @@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
> > > >                       goto resched;
> > > >               }
> > > >
> > > > -             ret = shrink_memcg(memcg);
> > > > +             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
> > > >               /* drop the extra reference */
> > > >               mem_cgroup_put(memcg);
> > > >
> > > > @@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
> > > >       objcg = get_obj_cgroup_from_folio(folio);
> > > >       if (objcg && !obj_cgroup_may_zswap(objcg)) {
> > > >               memcg = get_mem_cgroup_from_objcg(objcg);
> > > > -             if (shrink_memcg(memcg)) {
> > > > +             if (shrink_memcg(memcg, 1)) {
> > >
> > > Why 64 for the global limit but only 1 for the cgroup limit? That
> > > seems arbitrary in multiple ways.
> >
> > I suggested that we keep the writeback here without batching and do
> > that change separately, mainly out of abundance of caution as
> > writeback is done synchronously here so the extra latency could be
> > problematic. I think we probably want to measure the performance
> > impact of that separately.
> >
> > That being said, this path is potentially too expensive anyway due to
> > the flush, but I would rather we do some basic measurements before
> > batching here.
> >
> > What do you think?
>
> It's not an unknown, right? We know this works for direct reclaimers,
> cgroup limit reclaim e.g., and what the latency implications are.
>
> Because of how reclaim works, we also know it'll call zswap_store() in
> batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to
> each call shrink_memcg() once we're at the limit - while still risking
> rejections due to compressibility differences.
>
> My worry is that if we start with an inconsistency, we'll be stuck
> with it for a long time.
>
> I'd rather start with the clean, consistent version. Dial it back only
> if we have data to justfiy the complication that we can put into a
> comment and the changelog that outlines why exactly it's different.

I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH
as the batch size in shrink_memcg(), but I would be more comfortable
if we did some sanity testing.

Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH
used for all paths, and memory.zswap.max set in a way that induces
writeback? You can probably set memory.zswap.max to 1% of total memory
instead of the global pool limit and rerun the same test.

Re: [PATCH v2 2/2] mm/zswap: Support batch writeback in shrink_memcg()

[Johannes Weiner]

On Thu, Jul 23, 2026 at 09:39:35AM -0700, Yosry Ahmed wrote:
> On Thu, Jul 23, 2026 at 6:55 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Wed, Jul 22, 2026 at 09:52:18PM -0700, Yosry Ahmed wrote:
> > > On Wed, Jul 22, 2026 at 7:27 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > > >
> > > > On Fri, Jul 17, 2026 at 04:51:51PM +0800, Hao Jia wrote:
> > > > > @@ -1369,7 +1402,7 @@ static void shrink_worker(struct work_struct *w)
> > > > >                       goto resched;
> > > > >               }
> > > > >
> > > > > -             ret = shrink_memcg(memcg);
> > > > > +             ret = shrink_memcg(memcg, NR_ZSWAP_WB_BATCH);
> > > > >               /* drop the extra reference */
> > > > >               mem_cgroup_put(memcg);
> > > > >
> > > > > @@ -1493,7 +1526,7 @@ bool zswap_store(struct folio *folio)
> > > > >       objcg = get_obj_cgroup_from_folio(folio);
> > > > >       if (objcg && !obj_cgroup_may_zswap(objcg)) {
> > > > >               memcg = get_mem_cgroup_from_objcg(objcg);
> > > > > -             if (shrink_memcg(memcg)) {
> > > > > +             if (shrink_memcg(memcg, 1)) {
> > > >
> > > > Why 64 for the global limit but only 1 for the cgroup limit? That
> > > > seems arbitrary in multiple ways.
> > >
> > > I suggested that we keep the writeback here without batching and do
> > > that change separately, mainly out of abundance of caution as
> > > writeback is done synchronously here so the extra latency could be
> > > problematic. I think we probably want to measure the performance
> > > impact of that separately.
> > >
> > > That being said, this path is potentially too expensive anyway due to
> > > the flush, but I would rather we do some basic measurements before
> > > batching here.
> > >
> > > What do you think?
> >
> > It's not an unknown, right? We know this works for direct reclaimers,
> > cgroup limit reclaim e.g., and what the latency implications are.
> >
> > Because of how reclaim works, we also know it'll call zswap_store() in
> > batches of SWAP_CLUSTER_MAX. If we don't batch here, they're likely to
> > each call shrink_memcg() once we're at the limit - while still risking
> > rejections due to compressibility differences.
> >
> > My worry is that if we start with an inconsistency, we'll be stuck
> > with it for a long time.
> >
> > I'd rather start with the clean, consistent version. Dial it back only
> > if we have data to justfiy the complication that we can put into a
> > comment and the changelog that outlines why exactly it's different.
> 
> I am fine with doing that and basically always using NR_ZSWAP_WB_BATCH
> as the batch size in shrink_memcg(), but I would be more comfortable
> if we did some sanity testing.
> 
> Hao, would you be able to do some smoke testing with NR_ZSWAP_WB_BATCH
> used for all paths, and memory.zswap.max set in a way that induces
> writeback? You can probably set memory.zswap.max to 1% of total memory
> instead of the global pool limit and rerun the same test.

Please just use SWAP_CLUSTER_MAX. That's what the reclaim side which
issues the stores is also using for batching.