Re: [PATCH v2 3/3] mm/swap: skip relocation of tableless clusters

[Kairui Song]

On Tue, Nov 10, 2026 at 11:14 PM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
>
> A stale reclaimer can reach relocate_cluster() after another allocator
> re-isolates an emptied cluster and drops its locks to populate the table.
> The cluster has zero count, CLUSTER_FLAG_NONE and no table, so relocation
> can free it again and dereference NULL in the DEBUG_VM empty check.
>
>     CPU0                                    CPU1
>     Reclaimer                               Allocator
>     alloc_swap_scan_cluster()
>       cluster_reclaim_range()
>         spin_unlock(&ci->lock)
>         __try_to_reclaim_swap()
>           swap_cache_del_folio()
>             __swap_cache_del_folio()
>               __swap_cache_do_del_folio()
>                 __swap_cluster_free_entries()
>                   free_cluster()
>                     __free_cluster()
>                       swap_cluster_free_table()
>             swap_cluster_unlock(ci)
>                                             isolate_lock_cluster()
>                                               swap_cluster_populate()
>                                                 drop ci and outer locks
>                                                 swap_cluster_alloc_table()
>         spin_lock(&ci->lock)
>       relocate_cluster()
>         free_cluster()
>           __free_cluster()
>             swap_cluster_assert_empty()
>               __swap_table_get()  /* NULL table */
>
> Leave tableless clusters to the allocator under ci->lock. This also stops
> a stale reclaimer from returning a cluster to the free list while table
> population is pending, avoiding the resulting FREE-to-NONE state cycle.
> The existing cluster lock serializes the table check and relocation with
> population.
>
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/sashiko-outbox-203018@xxxxxxxxxx/
> Fixes: 07adc4cf1ecd ("mm, swap: implement dynamic allocation of swap table")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>
> ---
>  mm/swapfile.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 32639f65f13c..9fd62e4ca7a0 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -815,6 +815,13 @@ static void relocate_cluster(struct swap_info_struct *si,
>         if (cluster_is_discard(ci))
>                 return;
>
> +       /*
> +        * An allocator may still be preparing the table.
> +        * Leave the cluster to it.

It's more like the cluster is in an "isolated" state so we shouldn't
touch it, similar to the "discarding" status above, this comment also
looks to me. Huge thanks for fixing this!

> +        */
> +       if (!cluster_table_is_alloced(ci))
> +               return;
> +
>         if (!ci->count) {
>                 if (ci->flags != CLUSTER_FLAG_FREE)
>                         free_cluster(si, ci);
> --
> 2.53.0-Meta
>

Acked-by: Kairui Song <kasong@xxxxxxxxxxx>