Re: [PATCH v3 12/12] mm, swap: merge zeromap into swap table

[YoungJun Park]

On Tue, Apr 21, 2026 at 02:16:56PM +0800, Kairui Song via B4 Relay wrote:

Nice! LGTM

Reviewed-by: Youngjun Park <youngjun.park@xxxxxxx>

A few nitpicks follow. take them if you find them useful. :)

> +static inline void __swap_table_clear_zero(struct swap_cluster_info *ci,
> +					   unsigned int ci_off)
> +{
> +

trailing blank line. 

> +#if SWAP_TABLE_HAS_ZEROFLAG
> +	unsigned long swp_tb = __swap_table_get(ci, ci_off);
> +
> +	VM_WARN_ON(!swp_tb_is_countable(swp_tb));
> +	swp_tb &= ~SWP_TB_ZERO_FLAG;
> +	__swap_table_set(ci, ci_off, swp_tb);
> +#else
> +	lockdep_assert_held(&ci->lock);
> +	__clear_bit(ci_off, ci->zero_bitmap);
> +#endif
> +}
...
> +
>  	table = (struct swap_table *)rcu_access_pointer(ci->table);
>  	if (!table)
>  		return;
> @@ -470,6 +475,13 @@ static int swap_cluster_alloc_table(struct swap_cluster_info *ci, gfp_t gfp)
>  	if (!ci->memcg_table)
>  		ret = -ENOMEM;
>  #endif
> +
> +#if !SWAP_TABLE_HAS_ZEROFLAG
> +	ci->zero_bitmap = bitmap_zalloc(SWAPFILE_CLUSTER, gfp);
> +	if (!ci->zero_bitmap)
> +		ret = -ENOMEM;
> +#endif
> +

memcg_table above uses `if (!ci->memcg_table)` before
kzalloc, but ci->zero_bitmap is assigned unconditionally.  Both
are NULL on entry today (swap_cluster_free_table nullifies them),
so either form works but the asymmetry reads oddly.  Either
drop the memcg guard (with an entry VM_WARN_ON asserting both
NULL by design), or mirror the guard here.