Re: [RFC PATCH v2 3/3] mm/zsmalloc: drop class lock before freeing zspage
From: Joshua Hahn
Date: Thu May 28 2026 - 20:00:07 EST
On Wed, 27 May 2026 19:59:30 +0800 Wenchao Hao <haowenchao22@xxxxxxxxx> wrote:
> From: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>
>
> Currently in zs_free(), the class->lock is held until the zspage is
> completely freed and the counters are updated. However, freeing pages
> back to the buddy allocator requires acquiring the zone lock.
>
> Under heavy memory pressure, zone lock contention can be severe. When
> this happens, the CPU holding the class->lock will stall waiting for
> the zone lock, thereby blocking all other CPUs attempting to acquire
> the same class->lock.
>
> This patch shrinks the critical section of the class->lock to reduce
> lock contention. By moving the actual page freeing process outside the
> class->lock, we can improve the concurrency performance of zs_free().
Hello Wenchao and Xueyuan,
Thank you for the patch. This is a really cool idea and I think it makes
a lot of sense.
LGTM aside from a few nits, with those resolved please feel free to add
Reviewed-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
> fullness = fix_fullness_group(class, zspage);
> - if (fullness == ZS_INUSE_RATIO_0)
> - free_zspage(pool, class, zspage);
> + if (fullness == ZS_INUSE_RATIO_0) {
> + if (trylock_zspage(zspage)) {
> + remove_zspage(class, zspage);
> + class_stat_sub(class, ZS_OBJS_ALLOCATED,
> + class->objs_per_zspage);
> + zspage_to_free = zspage;
> + } else
> + kick_deferred_free(pool);
NIT: We should probably add opening and closing braces here as well since the
if() case is more than one line.
> + }
>
> spin_unlock(&class->lock);
Sashiko caught this in [1] and I was confused why the likely() doesn't make
sense here, but I finally realized that zspage_to_free is likely to be set
when comparing trylock_zspage() success vs. failure, but it's unlikely to
enter the ZS_INUSE_RATIO_0 case to begin with, so an unlikely() seems
correct here.
Thanks again. I hope you have a great day!
Joshua
> + if (likely(zspage_to_free)) {
> + __free_zspage_lockless(pool, zspage_to_free);
> + atomic_long_sub(class->pages_per_zspage, &pool->pages_allocated);
> + }
> cache_free_handle(handle);
> }
> EXPORT_SYMBOL_GPL(zs_free);
[1] https://sashiko.dev/#/patchset/20260527115930.3138213-1-haowenchao%40xiaomi.com