* [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
@ 2026-07-02 18:07 Yunzhao Li
2026-07-02 20:16 ` Johannes Weiner
2026-07-10 17:16 ` Nhat Pham
0 siblings, 2 replies; 14+ messages in thread
From: Yunzhao Li @ 2026-07-02 18:07 UTC (permalink / raw)
To: linux-mm
Cc: nphamcs, yosryahmed, shakeel.butt, hannes, hawk, akpm,
zhouchengming, Yunzhao Li
zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
global cgroup rstat lock synchronously. On machines with many CPUs and
NUMA nodes, this creates severe lock contention in the kswapd reclaim
path:
- Multiple kswapd threads (one per NUMA node) run concurrently.
- do_shrink_slab() invokes zswap_shrinker_count() for each
memcg-aware shrinker pass.
- Each call flushes the full cgroup rstat hierarchy under the global
lock.
On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
running production workloads with zswap enabled, perf shows 2.88% of
kernel cycles in osq_lock contention from this path:
2.88% [k] osq_lock
--__mutex_lock.constprop.0
--__cgroup_rstat_lock
--cgroup_rstat_flush_locked
--cgroup_rstat_flush
--zswap_shrinker_count
do_shrink_slab
shrink_slab
shrink_node
balance_pgdat
kswapd
84% of kswapd kernel cycles are spent in
shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
page reclaim (shrink_lruvec).
Controlled A/B on identical hardware and workload:
shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
shrinker=N: 0.00% osq_lock, memory PSI 0.57%
eBPF-based rstat lock wait measurement across 8 production metals
confirms the contention splits cleanly along shrinker enablement:
shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
zswap_shrinker_count() only produces a heuristic estimate, scaled by
compression ratio via mult_frac(). The actual writeback happens in
zswap_shrinker_scan(). Slightly stale stats are acceptable here.
Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
the periodic 2-second flusher is one full cycle late. This matches the
approach already used in prepare_scan_control() (mm/vmscan.c) for the
same reclaim path.
After applying this patch, rstat flush latency and lock wait time on
shrinker=Y machines dropped to the same level as shrinker=N controls,
while the zswap shrinker continues to function (pool size remains
bounded under the max_pool_percent cap).
Previously discussed:
- Chengming Zhou (Dec 2023): rstat contention from
zswap_shrinker_count [1]
- Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
flush [2]
- Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
flushers [3]
- Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
[1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
[2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
[3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
[4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
---
mm/zswap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 761cd699e..b5a17ea20 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1217,7 +1217,7 @@ static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
* Without memcg, use the zswap pool-wide metrics.
*/
if (!mem_cgroup_disabled()) {
- mem_cgroup_flush_stats(memcg);
+ mem_cgroup_flush_stats_ratelimited(memcg);
nr_backing = memcg_page_state(memcg, MEMCG_ZSWAP_B) >> PAGE_SHIFT;
nr_stored = memcg_page_state(memcg, MEMCG_ZSWAPPED);
} else {
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-02 18:07 [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count() Yunzhao Li
@ 2026-07-02 20:16 ` Johannes Weiner
2026-07-02 20:51 ` Jesper Dangaard Brouer
2026-07-06 19:46 ` Yosry Ahmed
2026-07-10 17:16 ` Nhat Pham
1 sibling, 2 replies; 14+ messages in thread
From: Johannes Weiner @ 2026-07-02 20:16 UTC (permalink / raw)
To: Yunzhao Li
Cc: linux-mm, nphamcs, yosryahmed, shakeel.butt, hawk, akpm,
zhouchengming
On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
> global cgroup rstat lock synchronously. On machines with many CPUs and
> NUMA nodes, this creates severe lock contention in the kswapd reclaim
> path:
>
> - Multiple kswapd threads (one per NUMA node) run concurrently.
> - do_shrink_slab() invokes zswap_shrinker_count() for each
> memcg-aware shrinker pass.
> - Each call flushes the full cgroup rstat hierarchy under the global
> lock.
>
> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
> running production workloads with zswap enabled, perf shows 2.88% of
> kernel cycles in osq_lock contention from this path:
>
> 2.88% [k] osq_lock
> --__mutex_lock.constprop.0
> --__cgroup_rstat_lock
> --cgroup_rstat_flush_locked
> --cgroup_rstat_flush
> --zswap_shrinker_count
> do_shrink_slab
> shrink_slab
> shrink_node
> balance_pgdat
> kswapd
>
> 84% of kswapd kernel cycles are spent in
> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
> page reclaim (shrink_lruvec).
>
> Controlled A/B on identical hardware and workload:
>
> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
>
> eBPF-based rstat lock wait measurement across 8 production metals
> confirms the contention splits cleanly along shrinker enablement:
>
> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
>
> zswap_shrinker_count() only produces a heuristic estimate, scaled by
> compression ratio via mult_frac(). The actual writeback happens in
> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
>
> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
> the periodic 2-second flusher is one full cycle late. This matches the
> approach already used in prepare_scan_control() (mm/vmscan.c) for the
> same reclaim path.
>
> After applying this patch, rstat flush latency and lock wait time on
> shrinker=Y machines dropped to the same level as shrinker=N controls,
> while the zswap shrinker continues to function (pool size remains
> bounded under the max_pool_percent cap).
>
> Previously discussed:
> - Chengming Zhou (Dec 2023): rstat contention from
> zswap_shrinker_count [1]
> - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
> flush [2]
> - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
> flushers [3]
> - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
>
> [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
> [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
> [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
> [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
>
> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
> Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
A lot can happen in 2s, but I agree doing this every time is
silly. vmscan has been good with 2s for a while, so this should be
fine as well. We can re-evaluate if we run into weird behavior.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-02 20:16 ` Johannes Weiner
@ 2026-07-02 20:51 ` Jesper Dangaard Brouer
2026-07-06 19:46 ` Yosry Ahmed
1 sibling, 0 replies; 14+ messages in thread
From: Jesper Dangaard Brouer @ 2026-07-02 20:51 UTC (permalink / raw)
To: Johannes Weiner, Yunzhao Li
Cc: linux-mm, nphamcs, yosryahmed, shakeel.butt, akpm, zhouchengming
On 02/07/2026 22.16, Johannes Weiner wrote:
> On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
>> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
>> global cgroup rstat lock synchronously. On machines with many CPUs and
>> NUMA nodes, this creates severe lock contention in the kswapd reclaim
>> path:
>>
>> - Multiple kswapd threads (one per NUMA node) run concurrently.
>> - do_shrink_slab() invokes zswap_shrinker_count() for each
>> memcg-aware shrinker pass.
>> - Each call flushes the full cgroup rstat hierarchy under the global
>> lock.
>>
>> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
>> running production workloads with zswap enabled, perf shows 2.88% of
>> kernel cycles in osq_lock contention from this path:
>>
>> 2.88% [k] osq_lock
>> --__mutex_lock.constprop.0
>> --__cgroup_rstat_lock
>> --cgroup_rstat_flush_locked
>> --cgroup_rstat_flush
>> --zswap_shrinker_count
>> do_shrink_slab
>> shrink_slab
>> shrink_node
>> balance_pgdat
>> kswapd
>>
>> 84% of kswapd kernel cycles are spent in
>> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
>> page reclaim (shrink_lruvec).
>>
>> Controlled A/B on identical hardware and workload:
>>
>> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
>> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
>>
>> eBPF-based rstat lock wait measurement across 8 production metals
>> confirms the contention splits cleanly along shrinker enablement:
>>
>> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
>> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
>>
>> zswap_shrinker_count() only produces a heuristic estimate, scaled by
>> compression ratio via mult_frac(). The actual writeback happens in
>> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
>>
>> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
>> the periodic 2-second flusher is one full cycle late. This matches the
>> approach already used in prepare_scan_control() (mm/vmscan.c) for the
>> same reclaim path.
>>
>> After applying this patch, rstat flush latency and lock wait time on
>> shrinker=Y machines dropped to the same level as shrinker=N controls,
>> while the zswap shrinker continues to function (pool size remains
>> bounded under the max_pool_percent cap).
>>
>> Previously discussed:
>> - Chengming Zhou (Dec 2023): rstat contention from
>> zswap_shrinker_count [1]
>> - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
>> flush [2]
>> - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
>> flushers [3]
>> - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
>>
>> [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
>> [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
>> [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
>> [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
>>
>> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>> Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
>> Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>
> A lot can happen in 2s, but I agree doing this every time is
> silly. vmscan has been good with 2s for a while, so this should be
> fine as well. We can re-evaluate if we run into weird behavior.
I don't know if an ACK from me is needed, but I'm just acknowledging
that I helped Yunzhao with developing this patch internally at Cloudflare.
Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
--Jesper
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-02 20:16 ` Johannes Weiner
2026-07-02 20:51 ` Jesper Dangaard Brouer
@ 2026-07-06 19:46 ` Yosry Ahmed
2026-07-08 9:10 ` Jesper Dangaard Brouer
1 sibling, 1 reply; 14+ messages in thread
From: Yosry Ahmed @ 2026-07-06 19:46 UTC (permalink / raw)
To: Johannes Weiner
Cc: Yunzhao Li, linux-mm, nphamcs, yosryahmed, shakeel.butt, hawk,
akpm, zhouchengming, James Houghton
On Thu, Jul 2, 2026 at 6:01 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
> > zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
> > global cgroup rstat lock synchronously. On machines with many CPUs and
> > NUMA nodes, this creates severe lock contention in the kswapd reclaim
> > path:
> >
> > - Multiple kswapd threads (one per NUMA node) run concurrently.
> > - do_shrink_slab() invokes zswap_shrinker_count() for each
> > memcg-aware shrinker pass.
> > - Each call flushes the full cgroup rstat hierarchy under the global
> > lock.
> >
> > On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
> > running production workloads with zswap enabled, perf shows 2.88% of
> > kernel cycles in osq_lock contention from this path:
> >
> > 2.88% [k] osq_lock
> > --__mutex_lock.constprop.0
> > --__cgroup_rstat_lock
> > --cgroup_rstat_flush_locked
> > --cgroup_rstat_flush
> > --zswap_shrinker_count
> > do_shrink_slab
> > shrink_slab
> > shrink_node
> > balance_pgdat
> > kswapd
> >
> > 84% of kswapd kernel cycles are spent in
> > shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
> > page reclaim (shrink_lruvec).
> >
> > Controlled A/B on identical hardware and workload:
> >
> > shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
> > shrinker=N: 0.00% osq_lock, memory PSI 0.57%
> >
> > eBPF-based rstat lock wait measurement across 8 production metals
> > confirms the contention splits cleanly along shrinker enablement:
> >
> > shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
> > shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
> >
> > zswap_shrinker_count() only produces a heuristic estimate, scaled by
> > compression ratio via mult_frac(). The actual writeback happens in
> > zswap_shrinker_scan(). Slightly stale stats are acceptable here.
> >
> > Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
> > the periodic 2-second flusher is one full cycle late. This matches the
> > approach already used in prepare_scan_control() (mm/vmscan.c) for the
> > same reclaim path.
> >
> > After applying this patch, rstat flush latency and lock wait time on
> > shrinker=Y machines dropped to the same level as shrinker=N controls,
> > while the zswap shrinker continues to function (pool size remains
> > bounded under the max_pool_percent cap).
> >
> > Previously discussed:
> > - Chengming Zhou (Dec 2023): rstat contention from
> > zswap_shrinker_count [1]
> > - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
> > flush [2]
> > - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
> > flushers [3]
> > - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
> >
> > [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
> > [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
> > [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
> > [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
> >
> > Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
> > Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> > Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
> > Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>
> A lot can happen in 2s, but I agree doing this every time is
> silly. vmscan has been good with 2s for a while, so this should be
> fine as well. We can re-evaluate if we run into weird behavior.
Hmm actually we are starting to exercise kernels with the 2s change in
the vmscan path, and we are observing some seemingly premature OOM
kills for VMs under memory pressure. We are still looking into it, but
I would honestly hold off on adding another ratelimited flush here. It
may not be as big of a deal for the zswap path, but I'd rather fix
this properly instead of just plugging it. We should probably move
away from using rstat for zswapped/zswap_b stats and use page counters
as we discussed previously (i.e. eliminating the flushes to begin
with).
(Adding James here as he's looking into the premature OOM kills internally)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-06 19:46 ` Yosry Ahmed
@ 2026-07-08 9:10 ` Jesper Dangaard Brouer
2026-07-08 19:32 ` Yosry Ahmed
0 siblings, 1 reply; 14+ messages in thread
From: Jesper Dangaard Brouer @ 2026-07-08 9:10 UTC (permalink / raw)
To: Yosry Ahmed, Johannes Weiner
Cc: Yunzhao Li, linux-mm, nphamcs, yosryahmed, shakeel.butt, akpm,
zhouchengming, James Houghton, kernel-team
On 06/07/2026 21.46, Yosry Ahmed wrote:
> On Thu, Jul 2, 2026 at 6:01 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>>
>> On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
>>> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
>>> global cgroup rstat lock synchronously. On machines with many CPUs and
>>> NUMA nodes, this creates severe lock contention in the kswapd reclaim
>>> path:
>>>
>>> - Multiple kswapd threads (one per NUMA node) run concurrently.
>>> - do_shrink_slab() invokes zswap_shrinker_count() for each
>>> memcg-aware shrinker pass.
>>> - Each call flushes the full cgroup rstat hierarchy under the global
>>> lock.
>>>
>>> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
>>> running production workloads with zswap enabled, perf shows 2.88% of
>>> kernel cycles in osq_lock contention from this path:
>>>
>>> 2.88% [k] osq_lock
>>> --__mutex_lock.constprop.0
>>> --__cgroup_rstat_lock
>>> --cgroup_rstat_flush_locked
>>> --cgroup_rstat_flush
>>> --zswap_shrinker_count
>>> do_shrink_slab
>>> shrink_slab
>>> shrink_node
>>> balance_pgdat
>>> kswapd
>>>
>>> 84% of kswapd kernel cycles are spent in
>>> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
>>> page reclaim (shrink_lruvec).
>>>
>>> Controlled A/B on identical hardware and workload:
>>>
>>> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
>>> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
>>>
>>> eBPF-based rstat lock wait measurement across 8 production metals
>>> confirms the contention splits cleanly along shrinker enablement:
>>>
>>> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
>>> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
>>>
>>> zswap_shrinker_count() only produces a heuristic estimate, scaled by
>>> compression ratio via mult_frac(). The actual writeback happens in
>>> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
>>>
>>> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
>>> the periodic 2-second flusher is one full cycle late. This matches the
>>> approach already used in prepare_scan_control() (mm/vmscan.c) for the
>>> same reclaim path.
>>>
>>> After applying this patch, rstat flush latency and lock wait time on
>>> shrinker=Y machines dropped to the same level as shrinker=N controls,
>>> while the zswap shrinker continues to function (pool size remains
>>> bounded under the max_pool_percent cap).
>>>
>>> Previously discussed:
>>> - Chengming Zhou (Dec 2023): rstat contention from
>>> zswap_shrinker_count [1]
>>> - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
>>> flush [2]
>>> - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
>>> flushers [3]
>>> - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
>>>
>>> [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
>>> [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
>>> [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
>>> [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
>>>
>>> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>> Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
>>> Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
>>
>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>>
>> A lot can happen in 2s, but I agree doing this every time is
>> silly. vmscan has been good with 2s for a while, so this should be
>> fine as well. We can re-evaluate if we run into weird behavior.
>
> Hmm actually we are starting to exercise kernels with the 2s change in
> the vmscan path, and we are observing some seemingly premature OOM
> kills for VMs under memory pressure. We are still looking into it, but
> I would honestly hold off on adding another ratelimited flush here. It
> may not be as big of a deal for the zswap path, but I'd rather fix
> this properly instead of just plugging it. We should probably move
> away from using rstat for zswapped/zswap_b stats and use page counters
> as we discussed previously (i.e. eliminating the flushes to begin
> with).
>
> (Adding James here as he's looking into the premature OOM kills internally)
IMHO it doesn't make any sense to hold off on this patch.
This only relates to zswap path with shrinker enabled. Today we are
forced to run with shrinker disabled.
If you find some better approach to ratelimited flush, then that is
great. You can change this code again for your use-case. No reason to
stall this patch IMHO.
--Jesper
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-08 9:10 ` Jesper Dangaard Brouer
@ 2026-07-08 19:32 ` Yosry Ahmed
2026-07-08 22:16 ` Yunzhao Li
0 siblings, 1 reply; 14+ messages in thread
From: Yosry Ahmed @ 2026-07-08 19:32 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Johannes Weiner, Yunzhao Li, linux-mm, nphamcs, yosryahmed,
shakeel.butt, akpm, zhouchengming, James Houghton, kernel-team
On Wed, Jul 8, 2026 at 2:11 AM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
>
>
>
> On 06/07/2026 21.46, Yosry Ahmed wrote:
> > On Thu, Jul 2, 2026 at 6:01 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >>
> >> On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
> >>> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
> >>> global cgroup rstat lock synchronously. On machines with many CPUs and
> >>> NUMA nodes, this creates severe lock contention in the kswapd reclaim
> >>> path:
> >>>
> >>> - Multiple kswapd threads (one per NUMA node) run concurrently.
> >>> - do_shrink_slab() invokes zswap_shrinker_count() for each
> >>> memcg-aware shrinker pass.
> >>> - Each call flushes the full cgroup rstat hierarchy under the global
> >>> lock.
> >>>
> >>> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
> >>> running production workloads with zswap enabled, perf shows 2.88% of
> >>> kernel cycles in osq_lock contention from this path:
> >>>
> >>> 2.88% [k] osq_lock
> >>> --__mutex_lock.constprop.0
> >>> --__cgroup_rstat_lock
> >>> --cgroup_rstat_flush_locked
> >>> --cgroup_rstat_flush
> >>> --zswap_shrinker_count
> >>> do_shrink_slab
> >>> shrink_slab
> >>> shrink_node
> >>> balance_pgdat
> >>> kswapd
> >>>
> >>> 84% of kswapd kernel cycles are spent in
> >>> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
> >>> page reclaim (shrink_lruvec).
> >>>
> >>> Controlled A/B on identical hardware and workload:
> >>>
> >>> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
> >>> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
> >>>
> >>> eBPF-based rstat lock wait measurement across 8 production metals
> >>> confirms the contention splits cleanly along shrinker enablement:
> >>>
> >>> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
> >>> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
> >>>
> >>> zswap_shrinker_count() only produces a heuristic estimate, scaled by
> >>> compression ratio via mult_frac(). The actual writeback happens in
> >>> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
> >>>
> >>> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
> >>> the periodic 2-second flusher is one full cycle late. This matches the
> >>> approach already used in prepare_scan_control() (mm/vmscan.c) for the
> >>> same reclaim path.
> >>>
> >>> After applying this patch, rstat flush latency and lock wait time on
> >>> shrinker=Y machines dropped to the same level as shrinker=N controls,
> >>> while the zswap shrinker continues to function (pool size remains
> >>> bounded under the max_pool_percent cap).
> >>>
> >>> Previously discussed:
> >>> - Chengming Zhou (Dec 2023): rstat contention from
> >>> zswap_shrinker_count [1]
> >>> - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
> >>> flush [2]
> >>> - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
> >>> flushers [3]
> >>> - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
> >>>
> >>> [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
> >>> [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
> >>> [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
> >>> [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
> >>>
> >>> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
> >>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> >>> Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
> >>> Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
> >>
> >> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> >>
> >> A lot can happen in 2s, but I agree doing this every time is
> >> silly. vmscan has been good with 2s for a while, so this should be
> >> fine as well. We can re-evaluate if we run into weird behavior.
> >
> > Hmm actually we are starting to exercise kernels with the 2s change in
> > the vmscan path, and we are observing some seemingly premature OOM
> > kills for VMs under memory pressure. We are still looking into it, but
> > I would honestly hold off on adding another ratelimited flush here. It
> > may not be as big of a deal for the zswap path, but I'd rather fix
> > this properly instead of just plugging it. We should probably move
> > away from using rstat for zswapped/zswap_b stats and use page counters
> > as we discussed previously (i.e. eliminating the flushes to begin
> > with).
> >
> > (Adding James here as he's looking into the premature OOM kills internally)
>
> IMHO it doesn't make any sense to hold off on this patch.
> This only relates to zswap path with shrinker enabled. Today we are
> forced to run with shrinker disabled.
>
> If you find some better approach to ratelimited flush, then that is
> great. You can change this code again for your use-case. No reason to
> stall this patch IMHO.
I think we should at least try a proper solution before falling back
to the whack-a-mole approach here. I don't like the ratelimited flush
because we use stale stats and the formulas start making less sense.
If it causes any problems, it's hard to reason about them. I agree
it's probably not gonna cause significant problems with the zswap
shrinker, but we should at least make an attempt at a proper fix
before resorting to this.
I *think* we should stop using rstat for MEMCG_ZSWAPPED and
MEMCG_ZSWAP_B. We can probably use per-memcg atomics for those, and
just iterate the parents and update the counters. It will also get rid
of the flush in obj_cgroup_may_zswap(), which is also problematic if
memory.zswap.max is used. Yes, it's more expensive, but I suspect it
may be just fine. The reclaim path is a slow path anyway. The fault
path is probably more concerning but we won't know unless we try.
rstat has a heavily optimized update path and a really expensive read
path, and I think that tradeoff just doesn't make much sense in the
zswap case.
Can we try doing that instead? If there aren't significant performance
regressions it would put us in a much better place imo.
Also, a curious question, the changelog says:
> while the zswap shrinker continues to function (pool size remains
> bounded under the max_pool_percent cap).
AFAICT the shrinker's goal is not to keep the pool size under the cap,
that should happen regardless of whether or not the shrinker is
enabled. The shrinker proactively writes back memory from zswap as
part of reclaim, rather than waiting for the limit (global or memcg)
to be hit. What disadvantages are you running into with disabling the
shrinker?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-08 19:32 ` Yosry Ahmed
@ 2026-07-08 22:16 ` Yunzhao Li
2026-07-09 0:08 ` Yosry Ahmed
0 siblings, 1 reply; 14+ messages in thread
From: Yunzhao Li @ 2026-07-08 22:16 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
> What disadvantages are you running into with disabling the shrinker?
We lose about 16 GiB of MemAvailable per machine. We A/B'd this on
the same hardware (EPYC 9684X, 96c/12 NUMA, 384 GiB), same workload:
shrinker=Y: MemAvailable ~75 GiB
shrinker=N: MemAvailable ~59 GiB
Based on my reading of the code, the zswap pool's zsmalloc pages
aren't on any LRU list, so kswapd can only reach them through the
shrinker. With shrinker=N the pool just grows until store-time
rejection or swapin frees entries.
Yunzhao
On Wed, Jul 8, 2026 at 12:33 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Wed, Jul 8, 2026 at 2:11 AM Jesper Dangaard Brouer <hawk@kernel.org> wrote:
> >
> >
> >
> > On 06/07/2026 21.46, Yosry Ahmed wrote:
> > > On Thu, Jul 2, 2026 at 6:01 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > >>
> > >> On Thu, Jul 02, 2026 at 11:07:35AM -0700, Yunzhao Li wrote:
> > >>> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
> > >>> global cgroup rstat lock synchronously. On machines with many CPUs and
> > >>> NUMA nodes, this creates severe lock contention in the kswapd reclaim
> > >>> path:
> > >>>
> > >>> - Multiple kswapd threads (one per NUMA node) run concurrently.
> > >>> - do_shrink_slab() invokes zswap_shrinker_count() for each
> > >>> memcg-aware shrinker pass.
> > >>> - Each call flushes the full cgroup rstat hierarchy under the global
> > >>> lock.
> > >>>
> > >>> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
> > >>> running production workloads with zswap enabled, perf shows 2.88% of
> > >>> kernel cycles in osq_lock contention from this path:
> > >>>
> > >>> 2.88% [k] osq_lock
> > >>> --__mutex_lock.constprop.0
> > >>> --__cgroup_rstat_lock
> > >>> --cgroup_rstat_flush_locked
> > >>> --cgroup_rstat_flush
> > >>> --zswap_shrinker_count
> > >>> do_shrink_slab
> > >>> shrink_slab
> > >>> shrink_node
> > >>> balance_pgdat
> > >>> kswapd
> > >>>
> > >>> 84% of kswapd kernel cycles are spent in
> > >>> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
> > >>> page reclaim (shrink_lruvec).
> > >>>
> > >>> Controlled A/B on identical hardware and workload:
> > >>>
> > >>> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
> > >>> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
> > >>>
> > >>> eBPF-based rstat lock wait measurement across 8 production metals
> > >>> confirms the contention splits cleanly along shrinker enablement:
> > >>>
> > >>> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
> > >>> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
> > >>>
> > >>> zswap_shrinker_count() only produces a heuristic estimate, scaled by
> > >>> compression ratio via mult_frac(). The actual writeback happens in
> > >>> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
> > >>>
> > >>> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
> > >>> the periodic 2-second flusher is one full cycle late. This matches the
> > >>> approach already used in prepare_scan_control() (mm/vmscan.c) for the
> > >>> same reclaim path.
> > >>>
> > >>> After applying this patch, rstat flush latency and lock wait time on
> > >>> shrinker=Y machines dropped to the same level as shrinker=N controls,
> > >>> while the zswap shrinker continues to function (pool size remains
> > >>> bounded under the max_pool_percent cap).
> > >>>
> > >>> Previously discussed:
> > >>> - Chengming Zhou (Dec 2023): rstat contention from
> > >>> zswap_shrinker_count [1]
> > >>> - Shakeel Butt (Aug 2024): zswap_shrinker_count still uses sync
> > >>> flush [2]
> > >>> - Yosry Ahmed (Aug 2024): suggested eliminating in-kernel
> > >>> flushers [3]
> > >>> - Jesper Dangaard Brouer (Sep 2024): cgroup/rstat V11 patch [4]
> > >>>
> > >>> [1] https://lore.kernel.org/linux-mm/20231206103935.3440502-1-zhouchengming@bytedance.com/
> > >>> [2] https://lore.kernel.org/linux-mm/CALvZod7LFxLCxVpOFH8b2Ppm8T40HPGMKQwX_=NPCWB_mFW+oQ@mail.gmail.com/
> > >>> [3] https://lore.kernel.org/linux-mm/CAJD7tkYvFyOSX+rP_FKGBhxvZiCDxtpsNp-c5CGOA-4Bq9oXSg@mail.gmail.com/
> > >>> [4] https://lore.kernel.org/linux-mm/172616070094.2055617.17676042522679701515.stgit@firesoul/
> > >>>
> > >>> Suggested-by: Jesper Dangaard Brouer <hawk@kernel.org>
> > >>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> > >>> Signed-off-by: Yunzhao Li <yunzhao@cloudflare.com>
> > >>> Tested-by: Yunzhao Li <yunzhao@cloudflare.com>
> > >>
> > >> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > >>
> > >> A lot can happen in 2s, but I agree doing this every time is
> > >> silly. vmscan has been good with 2s for a while, so this should be
> > >> fine as well. We can re-evaluate if we run into weird behavior.
> > >
> > > Hmm actually we are starting to exercise kernels with the 2s change in
> > > the vmscan path, and we are observing some seemingly premature OOM
> > > kills for VMs under memory pressure. We are still looking into it, but
> > > I would honestly hold off on adding another ratelimited flush here. It
> > > may not be as big of a deal for the zswap path, but I'd rather fix
> > > this properly instead of just plugging it. We should probably move
> > > away from using rstat for zswapped/zswap_b stats and use page counters
> > > as we discussed previously (i.e. eliminating the flushes to begin
> > > with).
> > >
> > > (Adding James here as he's looking into the premature OOM kills internally)
> >
> > IMHO it doesn't make any sense to hold off on this patch.
> > This only relates to zswap path with shrinker enabled. Today we are
> > forced to run with shrinker disabled.
> >
> > If you find some better approach to ratelimited flush, then that is
> > great. You can change this code again for your use-case. No reason to
> > stall this patch IMHO.
>
> I think we should at least try a proper solution before falling back
> to the whack-a-mole approach here. I don't like the ratelimited flush
> because we use stale stats and the formulas start making less sense.
> If it causes any problems, it's hard to reason about them. I agree
> it's probably not gonna cause significant problems with the zswap
> shrinker, but we should at least make an attempt at a proper fix
> before resorting to this.
>
> I *think* we should stop using rstat for MEMCG_ZSWAPPED and
> MEMCG_ZSWAP_B. We can probably use per-memcg atomics for those, and
> just iterate the parents and update the counters. It will also get rid
> of the flush in obj_cgroup_may_zswap(), which is also problematic if
> memory.zswap.max is used. Yes, it's more expensive, but I suspect it
> may be just fine. The reclaim path is a slow path anyway. The fault
> path is probably more concerning but we won't know unless we try.
> rstat has a heavily optimized update path and a really expensive read
> path, and I think that tradeoff just doesn't make much sense in the
> zswap case.
>
> Can we try doing that instead? If there aren't significant performance
> regressions it would put us in a much better place imo.
>
> Also, a curious question, the changelog says:
> > while the zswap shrinker continues to function (pool size remains
> > bounded under the max_pool_percent cap).
>
> AFAICT the shrinker's goal is not to keep the pool size under the cap,
> that should happen regardless of whether or not the shrinker is
> enabled. The shrinker proactively writes back memory from zswap as
> part of reclaim, rather than waiting for the limit (global or memcg)
> to be hit. What disadvantages are you running into with disabling the
> shrinker?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-08 22:16 ` Yunzhao Li
@ 2026-07-09 0:08 ` Yosry Ahmed
2026-07-09 16:34 ` Yunzhao Li
0 siblings, 1 reply; 14+ messages in thread
From: Yosry Ahmed @ 2026-07-09 0:08 UTC (permalink / raw)
To: Yunzhao Li
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
On Wed, Jul 8, 2026 at 3:16 PM Yunzhao Li <yunzhao@cloudflare.com> wrote:
>
> > What disadvantages are you running into with disabling the shrinker?
>
> We lose about 16 GiB of MemAvailable per machine. We A/B'd this on
> the same hardware (EPYC 9684X, 96c/12 NUMA, 384 GiB), same workload:
>
> shrinker=Y: MemAvailable ~75 GiB
> shrinker=N: MemAvailable ~59 GiB
>
> Based on my reading of the code, the zswap pool's zsmalloc pages
> aren't on any LRU list, so kswapd can only reach them through the
> shrinker. With shrinker=N the pool just grows until store-time
> rejection or swapin frees entries.
Thanks for sharing the data, it's actually good to know that the
shrinker is resulting in significant savings. Are you observing any
performance impact when enabling it (e.g. higher fault latencies)?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-09 0:08 ` Yosry Ahmed
@ 2026-07-09 16:34 ` Yunzhao Li
2026-07-09 19:32 ` Yosry Ahmed
0 siblings, 1 reply; 14+ messages in thread
From: Yunzhao Li @ 2026-07-09 16:34 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
> Are you observing any performance impact when enabling it
> (e.g. higher fault latencies)?
Actually the opposite with this patch. Swap performance is better with the
shrinker enabled:
shrinker=Y: sync swap overhead 10.3 ms, swap rate ~800/s
shrinker=N: sync swap overhead 22.5 ms, swap rate ~1.7K/s
Per-swap latency is about the same (~30 µs P99 both ways), but
with shrinker=N there's roughly 2x more swap activity. I think
the extra 16 GiB of free RAM from the shrinker just means less
memory pressure overall, so fewer pages need to be swapped in.
The shrinker seems to do a good job picking entries that don't
get faulted back in under our Prod environment.
Yunzhao
On Wed, Jul 8, 2026 at 5:09 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Wed, Jul 8, 2026 at 3:16 PM Yunzhao Li <yunzhao@cloudflare.com> wrote:
> >
> > > What disadvantages are you running into with disabling the shrinker?
> >
> > We lose about 16 GiB of MemAvailable per machine. We A/B'd this on
> > the same hardware (EPYC 9684X, 96c/12 NUMA, 384 GiB), same workload:
> >
> > shrinker=Y: MemAvailable ~75 GiB
> > shrinker=N: MemAvailable ~59 GiB
> >
> > Based on my reading of the code, the zswap pool's zsmalloc pages
> > aren't on any LRU list, so kswapd can only reach them through the
> > shrinker. With shrinker=N the pool just grows until store-time
> > rejection or swapin frees entries.
>
> Thanks for sharing the data, it's actually good to know that the
> shrinker is resulting in significant savings. Are you observing any
> performance impact when enabling it (e.g. higher fault latencies)?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-09 16:34 ` Yunzhao Li
@ 2026-07-09 19:32 ` Yosry Ahmed
2026-07-10 16:35 ` Yunzhao Li
0 siblings, 1 reply; 14+ messages in thread
From: Yosry Ahmed @ 2026-07-09 19:32 UTC (permalink / raw)
To: Yunzhao Li
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
On Thu, Jul 9, 2026 at 9:34 AM Yunzhao Li <yunzhao@cloudflare.com> wrote:
>
> > Are you observing any performance impact when enabling it
> > (e.g. higher fault latencies)?
>
> Actually the opposite with this patch. Swap performance is better with the
> shrinker enabled:
>
> shrinker=Y: sync swap overhead 10.3 ms, swap rate ~800/s
> shrinker=N: sync swap overhead 22.5 ms, swap rate ~1.7K/s
>
> Per-swap latency is about the same (~30 µs P99 both ways), but
> with shrinker=N there's roughly 2x more swap activity. I think
> the extra 16 GiB of free RAM from the shrinker just means less
> memory pressure overall, so fewer pages need to be swapped in.
> The shrinker seems to do a good job picking entries that don't
> get faulted back in under our Prod environment.
Yeah the per-fault latency is exactly what I was looking for. Seems
like most of the faults are not faulting in the pages written back to
disk, nice.
Would you be able to take a stab at eliminating the flushes as I
suggested above as an alternative?
(Also in general, please avoid top-posting and respond inline:
https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-09 19:32 ` Yosry Ahmed
@ 2026-07-10 16:35 ` Yunzhao Li
2026-07-10 18:27 ` Yosry Ahmed
0 siblings, 1 reply; 14+ messages in thread
From: Yunzhao Li @ 2026-07-10 16:35 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
> Yeah the per-fault latency is exactly what I was looking for. Seems
> like most of the faults are not faulting in the pages written back to
> disk, nice.
Yeah, at least for our workloads the cold entries seem to
stay cold.
> Would you be able to take a stab at eliminating the flushes as I
> suggested above as an alternative?
memcg internals are a bit out of my depth to be honest. I can
definitely test a patch on our fleet though and get it verified
if someone picks it up.
The ratelimited fix in mm-new is working well for us in the
meantime, so it'd be great to keep that moving forward.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-02 18:07 [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count() Yunzhao Li
2026-07-02 20:16 ` Johannes Weiner
@ 2026-07-10 17:16 ` Nhat Pham
1 sibling, 0 replies; 14+ messages in thread
From: Nhat Pham @ 2026-07-10 17:16 UTC (permalink / raw)
To: Yunzhao Li
Cc: linux-mm, yosryahmed, shakeel.butt, hannes, hawk, akpm,
zhouchengming
On Thu, Jul 2, 2026 at 11:10 AM Yunzhao Li <yunzhao@cloudflare.com> wrote:
>
> zswap_shrinker_count() calls mem_cgroup_flush_stats(), which takes the
> global cgroup rstat lock synchronously. On machines with many CPUs and
> NUMA nodes, this creates severe lock contention in the kswapd reclaim
> path:
>
> - Multiple kswapd threads (one per NUMA node) run concurrently.
> - do_shrink_slab() invokes zswap_shrinker_count() for each
> memcg-aware shrinker pass.
> - Each call flushes the full cgroup rstat hierarchy under the global
> lock.
>
> On AMD EPYC 9684X machines (96 cores, 192 threads, 12 NUMA nodes)
> running production workloads with zswap enabled, perf shows 2.88% of
> kernel cycles in osq_lock contention from this path:
>
> 2.88% [k] osq_lock
> --__mutex_lock.constprop.0
> --__cgroup_rstat_lock
> --cgroup_rstat_flush_locked
> --cgroup_rstat_flush
> --zswap_shrinker_count
> do_shrink_slab
> shrink_slab
> shrink_node
> balance_pgdat
> kswapd
>
> 84% of kswapd kernel cycles are spent in
> shrink_slab -> zswap_shrinker_count -> cgroup_rstat_flush, not in actual
> page reclaim (shrink_lruvec).
>
> Controlled A/B on identical hardware and workload:
>
> shrinker=Y: 2.88% osq_lock, memory PSI 1.58%
> shrinker=N: 0.00% osq_lock, memory PSI 0.57%
>
> eBPF-based rstat lock wait measurement across 8 production metals
> confirms the contention splits cleanly along shrinker enablement:
>
> shrinker=Y: 50-250x more contended lock acquisitions (248/s vs 1.1/s)
> shrinker=N: baseline lock wait (0.0017 s/s vs 1.04 s/s)
>
> zswap_shrinker_count() only produces a heuristic estimate, scaled by
> compression ratio via mult_frac(). The actual writeback happens in
> zswap_shrinker_scan(). Slightly stale stats are acceptable here.
>
> Switch to mem_cgroup_flush_stats_ratelimited(), which only flushes if
> the periodic 2-second flusher is one full cycle late. This matches the
> approach already used in prepare_scan_control() (mm/vmscan.c) for the
> same reclaim path.
>
> After applying this patch, rstat flush latency and lock wait time on
> shrinker=Y machines dropped to the same level as shrinker=N controls,
> while the zswap shrinker continues to function (pool size remains
> bounded under the max_pool_percent cap).
I think I'm fine with this. It's been awhile and I no longer have the
perf trace, but this (the flushing) was one of the more expensive
parts when I was experimenting with zswap shrinker on a production
service.
If memory (and my brief revision of the codebase) serves, these two
stats are only used to estimate the compressibility of a workload.
Yes, a lot can happen in the duration induced by the ratelimiting, but
from what I have observed in production, compression ratio tends to
remain fairly stable in well-behaved systems :)
With all of that pre-amble out of the way, this looks good to me.
Thank you for fixing this, and please feel free to add this:
Acked-by: Nhat Pham <nphamcs@gmail.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-10 16:35 ` Yunzhao Li
@ 2026-07-10 18:27 ` Yosry Ahmed
2026-07-10 21:25 ` Yunzhao Li
0 siblings, 1 reply; 14+ messages in thread
From: Yosry Ahmed @ 2026-07-10 18:27 UTC (permalink / raw)
To: Yunzhao Li
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
On Fri, Jul 10, 2026 at 9:36 AM Yunzhao Li <yunzhao@cloudflare.com> wrote:
>
> > Yeah the per-fault latency is exactly what I was looking for. Seems
> > like most of the faults are not faulting in the pages written back to
> > disk, nice.
>
> Yeah, at least for our workloads the cold entries seem to
> stay cold.
>
>
> > Would you be able to take a stab at eliminating the flushes as I
> > suggested above as an alternative?
>
> memcg internals are a bit out of my depth to be honest. I can
> definitely test a patch on our fleet though and get it verified
> if someone picks it up.
I can try to come up with a patch if you can help with testing, but it
won't be for a week or two. How long would it generally take to verify
such a patch?
> The ratelimited fix in mm-new is working well for us in the
> meantime, so it'd be great to keep that moving forward.
Hmm we can keep it in the pipeline for now and perhaps drop it if we
can get a proper fix before the next merge window. Does this sound
good to you?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count()
2026-07-10 18:27 ` Yosry Ahmed
@ 2026-07-10 21:25 ` Yunzhao Li
0 siblings, 0 replies; 14+ messages in thread
From: Yunzhao Li @ 2026-07-10 21:25 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Jesper Dangaard Brouer, Johannes Weiner, linux-mm, nphamcs,
yosryahmed, shakeel.butt, akpm, zhouchengming, James Houghton,
kernel-team
On Fri, Jul 10, 2026 at 11:27 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Fri, Jul 10, 2026 at 9:36 AM Yunzhao Li <yunzhao@cloudflare.com> wrote:
> >
> > > Yeah the per-fault latency is exactly what I was looking for. Seems
> > > like most of the faults are not faulting in the pages written back to
> > > disk, nice.
> >
> > Yeah, at least for our workloads the cold entries seem to
> > stay cold.
> >
> >
> > > Would you be able to take a stab at eliminating the flushes as I
> > > suggested above as an alternative?
> >
> > memcg internals are a bit out of my depth to be honest. I can
> > definitely test a patch on our fleet though and get it verified
> > if someone picks it up.
>
> I can try to come up with a patch if you can help with testing, but it
> won't be for a week or two. How long would it generally take to verify
> such a patch?
Testing a couple of days once we have the patch. The main wrinkle is we're
still on 6.18, so depending on how much the memcg code has changed
it might take a bit longer to backport.
> > The ratelimited fix in mm-new is working well for us in the
> > meantime, so it'd be great to keep that moving forward.
>
> Hmm we can keep it in the pipeline for now and perhaps drop it if we
> can get a proper fix before the next merge window. Does this sound
> good to you?
Sounds good to me.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-07-10 21:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02 18:07 [PATCH] mm/zswap: use ratelimited stats flush in zswap_shrinker_count() Yunzhao Li
2026-07-02 20:16 ` Johannes Weiner
2026-07-02 20:51 ` Jesper Dangaard Brouer
2026-07-06 19:46 ` Yosry Ahmed
2026-07-08 9:10 ` Jesper Dangaard Brouer
2026-07-08 19:32 ` Yosry Ahmed
2026-07-08 22:16 ` Yunzhao Li
2026-07-09 0:08 ` Yosry Ahmed
2026-07-09 16:34 ` Yunzhao Li
2026-07-09 19:32 ` Yosry Ahmed
2026-07-10 16:35 ` Yunzhao Li
2026-07-10 18:27 ` Yosry Ahmed
2026-07-10 21:25 ` Yunzhao Li
2026-07-10 17:16 ` Nhat Pham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox