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

[Nhat Pham]

On Mon, Oct 19, 2026 at 9:06 AM Nhat Pham <nphamcs@xxxxxxxxx> 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()
>
> 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>

I'm revisiting the lifecycle of clusters, because vswap introduced
dynamic (i.e run-time freeing) behavior to it. For the most part it
looks good (both physical and virtual side), but this look racy to me.
I don't see how it can be safe. Once you drop the lock, all bets are
off...

But at the same time, I have not been able to reproduce it (yet). Will
report here if I manage to.

(Side note: my vswap code is actually safe in this scenario. CPU0
allocates a new cluster entirely, and lock it before publishing it. So
even if the cluster index is cached in CPU1, this race cannot happen).