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