Re: [RFC PATCH 03/11] mm, swap: add xswap cluster grow via VM_SPARSE vmalloc

[Nhat Pham]

On Mon, Jul 27, 2026 at 7:05 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
>
> Implement dynamic cluster_info array growth for xswap devices using a
> VM_SPARSE vmalloc area:
>
> 1. xswap_map_clusters(): Allocate physical pages and map them into
>    the pre-reserved VM_SPARSE KVA region via vm_area_map_pages().
>
> 2. xswap_unmap_clusters(): Unmap pages from the VM_SPARSE area via
>    vm_area_unmap_pages() (used by the error/teardown paths, shrink
>    comes later).
>
> 3. setup_swap_clusters_info() xswap path: Use get_vm_area(VM_SPARSE)
>    for the cluster_info array, lazily mapping only the initial chunk.
>
> 4. free_swap_cluster_info(): Refactor to take swap_info_struct*.
>    For xswap, unmap all clusters and free_vm_area().
>
> 5. swapoff: Remove snapshot locals; move p->max/p->cluster_info
>    clearing after free_swap_cluster_info().
>
> Signed-off-by: Baoquan He <baoquan.he@xxxxxxxxx>

I'll give my 2 cents on the direction we're pursuing later. I've done
a lot of thinking over the past weeks, and I have some new concerns
now regarding how we plan to land this joint venture :)

That said, this is a new piece of extension, done per my request
(kernel-driven dynamic swap address space growth - thanks for taking
my concerns seriously), and I've also evaluated this vmalloc-based
data structure in the past week, so I'll put in my inquiries/concerns.

> ---
>  mm/swapfile.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 216 insertions(+), 12 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 143088dae07d..6d9c95ed09bd 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -49,6 +49,24 @@
>  #include "internal.h"
>  #include "swap.h"
>
> +#ifdef CONFIG_XSWAP
> +/*
> + * xswap: dynamically grow the cluster_info array via a VM_SPARSE area.
> + *
> + * XSWAP_GROW_CLUSTERS is the number of clusters to map in one grow
> + * operation.  It is set to the number of cluster_info structs that
> + * fit in a single page (at least 16), so that the vmalloc page table
> + * overhead is proportional to the number of clusters mapped.
> + */
> +#define XSWAP_GROW_CLUSTERS \
> +       max_t(unsigned long, PAGE_SIZE / sizeof(struct swap_cluster_info), 16)
> +
> +static int xswap_map_clusters(struct swap_info_struct *si,
> +                             unsigned long start_idx, unsigned long nr);
> +static void xswap_unmap_clusters(struct swap_info_struct *si,
> +                                unsigned long start_idx, unsigned long nr);
> +#endif
> +
>  static void swap_range_alloc(struct swap_info_struct *si,
>                              unsigned int nr_entries);
>  static bool folio_swapcache_freeable(struct folio *folio);
> @@ -3041,20 +3059,47 @@ static void wait_for_allocation(struct swap_info_struct *si)
>
>         BUG_ON(si->flags & SWP_WRITEOK);
>
> +#ifdef CONFIG_XSWAP
> +       /*
> +        * xswap clusters beyond nr_clusters_mapped have been unmapped
> +        * by the shrinker and their vmalloc pages are no longer
> +        * accessible.  Only iterate over currently mapped clusters.
> +        */
> +       if (si->flags & SWP_XSWAP)
> +               end = min(end, READ_ONCE(si->nr_clusters_mapped) *
> +                         SWAPFILE_CLUSTER);
> +#endif
> +
>         for (offset = 0; offset < end; offset += SWAPFILE_CLUSTER) {
>                 ci = swap_cluster_lock(si, offset);
>                 swap_cluster_unlock(ci);
>         }
>  }
>
> -static void free_swap_cluster_info(struct swap_cluster_info *cluster_info,
> -                                  unsigned long maxpages)
> +static void free_swap_cluster_info(struct swap_info_struct *si)
>  {
> +       struct swap_cluster_info *cluster_info = si->cluster_info;
> +       unsigned long maxpages = si->max;
>         struct swap_cluster_info *ci;
> -       int i, nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
> +       int i, nr_clusters;
>
>         if (!cluster_info)
>                 return;
> +
> +#ifdef CONFIG_XSWAP
> +       if (si->flags & SWP_XSWAP) {
> +               /* Unmap all mapped clusters and free the VM_SPARSE area */
> +               if (si->nr_clusters_mapped > 0)
> +                       xswap_unmap_clusters(si, 0, si->nr_clusters_mapped);
> +               free_vm_area(si->cluster_vm);
> +               si->cluster_vm = NULL;
> +               si->nr_clusters = 0;
> +               si->nr_clusters_mapped = 0;
> +               return;
> +       }
> +#endif
> +
> +       nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER);
>         for (i = 0; i < nr_clusters; i++) {
>                 ci = cluster_info + i;
>                 /* Cluster with bad marks count will have a remaining table */
> @@ -3093,11 +3138,9 @@ static void flush_percpu_swap_cluster(struct swap_info_struct *si)
>  SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>  {
>         struct swap_info_struct *p = NULL;
> -       struct swap_cluster_info *cluster_info;
>         struct file *swap_file, *victim;
>         struct address_space *mapping;
>         struct inode *inode;
> -       unsigned int maxpages;
>         int err, found = 0;
>
>         if (!capable(CAP_SYS_ADMIN))
> @@ -3189,10 +3232,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>
>         swap_file = p->swap_file;
>         p->swap_file = NULL;
> -       maxpages = p->max;
> -       cluster_info = p->cluster_info;
> -       p->max = 0;
> -       p->cluster_info = NULL;
>         spin_unlock(&p->lock);
>         spin_unlock(&swap_lock);
>         arch_swap_invalidate_area(p->type);
> @@ -3200,7 +3239,9 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>         mutex_unlock(&swapon_mutex);
>         kfree(p->global_cluster);
>         p->global_cluster = NULL;
> -       free_swap_cluster_info(cluster_info, maxpages);
> +       free_swap_cluster_info(p);
> +       p->max = 0;
> +       p->cluster_info = NULL;
>
>         inode = mapping->host;
>
> @@ -3564,6 +3605,112 @@ static unsigned long read_swap_header(struct swap_info_struct *si,
>         return maxpages;
>  }
>
> +#ifdef CONFIG_XSWAP
> +static int xswap_map_clusters(struct swap_info_struct *si,
> +                             unsigned long start_idx, unsigned long nr)
> +{
> +       unsigned long start_addr = (unsigned long)si->cluster_info +
> +                                  (size_t)start_idx * sizeof(struct swap_cluster_info);
> +       unsigned long end_addr = start_addr + (size_t)nr * sizeof(struct swap_cluster_info);
> +       /*
> +        * vm_area_map_pages() requires that start and end be page-aligned.
> +        * If start_addr falls within a page that was already mapped by a
> +        * previous batch (grow path), round it up to skip the already-mapped
> +        * partial page.  Always round end_addr up so the vmap page table walk
> +        * terminates correctly (the walk loop exits when addr == end, and addr
> +        * advances by PAGE_SIZE each iteration).
> +        */
> +       unsigned long vm_start = PAGE_ALIGN(start_addr);
> +       unsigned long vm_end = PAGE_ALIGN(end_addr);
> +       unsigned long npages;
> +       struct page **pages;
> +       unsigned long i;
> +
> +       if (vm_start >= vm_end) {
> +               /* All requested clusters fall within already-mapped pages. */
> +               for (i = start_idx; i < start_idx + nr; i++)
> +                       spin_lock_init(&si->cluster_info[i].lock);
> +               WRITE_ONCE(si->nr_clusters_mapped, start_idx + nr);
> +               return 0;
> +       }
> +
> +       npages = (vm_end - vm_start) >> PAGE_SHIFT;
> +
> +       pages = kmalloc_array(npages, sizeof(*pages), GFP_KERNEL);
> +       if (!pages)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < npages; i++) {
> +               /*
> +                * __GFP_ZERO is critical: cluster_info structs contain pointer
> +                * fields (extend_table, zero_bitmap, memcg_table, table) that
> +                * must start as NULL.  Without zeroing, stale data from a
> +                * previous user of the page would look like valid pointers.
> +                */
> +               pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> +               if (!pages[i])
> +                       goto fail;
> +       }
> +
> +       if (vm_area_map_pages(si->cluster_vm, vm_start, vm_end, pages)) {
> +               i = npages; /* free all pages on failure */
> +               goto fail;

I was evaluating whether your vmalloc array can be extended to support
kernel-driven dynamic growth at least (either as a slot-in replacement
for xarray, or as a follow-up optimization if it's too complicated),
and I stumble this mapping action.

Seems like it does not take any GFP flag argument, and under the hood
it calls GFP_KERNEL. Would this be safe in the swap allocation path
(where you slotted it in in patch 4)? Kairui used a more precise set
of flags in this path, for e.g for swap table allocation:

ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC |
   GFP_KERNEL);


I'm guessing this has to do with the fact that we often entered
swapping out paths with PF_MEMALLOC... Is there any risk of deadlock
etc.?

Kairui, any thoughts?