Re: [PATCH] mm, swap: revalidate populated clusters before scanning

[Youngjun Park]

On Mon, Oct 19, 2026 at 09:06:15AM -0700, Nhat Pham wrote:
> swap_cluster_populate() can publish a cluster's table with the cluster
> lock dropped. A cached allocator can use and empty it before the
> populating allocator reacquires the lock. The resumed scan can then
> dereference a NULL table.
> 
> For example, CPU1 can retain a cached cluster offset on an SSD with
> per-page discard disabled:
> 
>     CPU0                          CPU1
>     ----                          ----
>     swap_cluster_populate()
>       swap_cluster_alloc_table() fails
>       spin_unlock(&ci->lock)
>       local_unlock(&percpu_swap_cluster.lock)
>       swap_cluster_alloc_table()
>         rcu_assign_pointer(ci->table, table)
>                                   swap_alloc_fast()
>                                   local_unlock(&percpu_swap_cluster.lock)
>                                   mem_cgroup_try_charge_swap() fails
>                                   swap_cache_del_folio()
>                                     __swap_cluster_free_entries()
>                                       free_cluster()
>                                         __free_cluster()
>                                           swap_cluster_free_table()
>                                             rcu_assign_pointer(..., NULL)
>       local_lock(&percpu_swap_cluster.lock)
>       spin_lock(&ci->lock)
>       return ci
>     alloc_swap_scan_cluster()
>       cluster_scan_range()
>         __swap_table_get()

Hello Nhat.

Good catch!

> Reject an unusable cluster under its lock before scanning it. Relocate
> it only while its table is present.
> 
> Fixes: cdd77f84d966 ("mm, swap: consolidate cluster allocation helpers")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>
> ---
>  mm/swapfile.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index c3288910b3e3..988b7316e0a1 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -981,7 +981,11 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
>  	bool need_reclaim, ret, usable;
>  
>  	lockdep_assert_held(&ci->lock);
> -	VM_WARN_ON(!cluster_is_usable(ci, order));
> +	if (!cluster_is_usable(ci, order)) {
> +		if (cluster_table_is_alloced(ci))
> +			relocate_cluster(si, ci);
> +		goto out_unlock;
> +	}

If this patch is applied as bloew, does it prevent the issue described here?

https://lore.kernel.org/linux-mm/20260720071342.50742-2-shikemeng@xxxxxxxxxxxxxxx/
(This patch publishes cluster table after lock acquired.)

This patch is chagned the way I suggested during review... :(

As you pointed out, another context can free the table 
without alloc context knwoing this situation.  
I missed this situation.

IMHO, Publishing the table only after reacquiring the cluster lock, as Kemeng
suggested, and rejecting an unusable cluster under the lock as in this
patch both look correct to me!

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