Re: [PATCH v3 03/14] mm, swap: refactor free_swap_cluster_info to take swap_info_struct

[Chris Li]

On Wed, Sep 16, 2026 at 12:20 AM Baoquan He <hebaoquan@xxxxxxxxxx> wrote:
>
> Change free_swap_cluster_info() to take struct swap_info_struct* instead
> of (cluster_info, maxpages). It now extracts the fields from si and
> clears si->cluster_info after freeing to avoid a double free on the
> swapon() error path.
>
> The new parameter also lets the xswap path access si->flags in the
> function.
>
> Signed-off-by: Baoquan He <hebaoquan@xxxxxxxxxx>

Reviewed-by: Chris Li <chrisl@xxxxxxxxxx>

Chris

> ---
>  mm/swapfile.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 13ae681acf07..4b260f9760ac 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3142,14 +3142,17 @@ static void wait_for_allocation(struct swap_info_struct *si)
>         }
>  }
>
> -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;
> +
> +       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 */
> @@ -3161,6 +3164,7 @@ static void free_swap_cluster_info(struct swap_cluster_info *cluster_info,
>                 spin_unlock(&ci->lock);
>         }
>         kvfree(cluster_info);
> +       si->cluster_info = NULL;
>  }
>
>  /*
> @@ -3188,11 +3192,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))
> @@ -3284,10 +3286,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);
> @@ -3295,7 +3293,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;
>
> @@ -3662,6 +3662,8 @@ static int setup_swap_clusters_info(struct swap_info_struct *si,
>         if (!cluster_info)
>                 goto err;
>
> +       si->cluster_info = cluster_info;
> +
>         for (i = 0; i < nr_clusters; i++)
>                 spin_lock_init(&cluster_info[i].lock);
>
> @@ -3725,7 +3727,7 @@ static int setup_swap_clusters_info(struct swap_info_struct *si,
>         si->cluster_info = cluster_info;
>         return 0;
>  err:
> -       free_swap_cluster_info(cluster_info, maxpages);
> +       free_swap_cluster_info(si);
>         return err;
>  }
>
> @@ -3947,7 +3949,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>         si->global_cluster = NULL;
>         inode = NULL;
>         destroy_swap_extents(si, swap_file);
> -       free_swap_cluster_info(si->cluster_info, si->max);
> +       free_swap_cluster_info(si);
>         si->cluster_info = NULL;
>         /*
>          * Clear the SWP_USED flag after all resources are freed so
> --
> 2.54.0
>