Re: [PATCH v3 10/14] mm, swap: refactor swapoff and add xswap_destroy

[Chris Li]

On Wed, Sep 16, 2026 at 12:21 AM Baoquan He <hebaoquan@xxxxxxxxxx> wrote:
>
> Extract __swapoff() from sys_swapoff() so the teardown logic can be
> shared, and make it work for file-less devices.
>
> Add xswap_destroy() to tear down a file-less xswap device by swap type,
> exposed via /sys/kernel/mm/xswap/destroy. Writing a swap type tears down
> that device; it requires CAP_SYS_ADMIN.
>
> Signed-off-by: Baoquan He <hebaoquan@xxxxxxxxxx>
> ---
>  mm/swapfile.c | 205 +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 151 insertions(+), 54 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 351c68bcd70b..cdcbcbaa6d87 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -73,6 +73,7 @@ static int xswap_mapped_end(pte_t *pte, unsigned long addr, void *data);
>  static void xswap_try_shrink(struct swap_info_struct *si);
>
>  static int xswap_create(int prio);
> +static int xswap_destroy(int type);
>
>  static ssize_t xswap_create_store(struct kobject *kobj,
>                                   struct kobj_attribute *attr,
> @@ -103,8 +104,35 @@ static ssize_t xswap_create_store(struct kobject *kobj,
>  static struct kobj_attribute xswap_create_attr = __ATTR(create, 0200, NULL,
>                                                         xswap_create_store);
>
> +static ssize_t xswap_destroy_store(struct kobject *kobj,
> +                                  struct kobj_attribute *attr,
> +                                  const char *buf, size_t count)
> +{
> +       unsigned long type;
> +       int err;
> +
> +       if (!capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       err = kstrtoul(buf, 0, &type);
> +       if (err)
> +               return err;
> +       if (type >= MAX_SWAPFILES)
> +               return -EINVAL;
> +
> +       err = xswap_destroy(type);
> +       if (err)
> +               return err;
> +
> +       return count;
> +}
> +
> +static struct kobj_attribute xswap_destroy_attr = __ATTR(destroy, 0200, NULL,
> +                                                        xswap_destroy_store);
> +
>  static struct attribute *xswap_attrs[] = {
>         &xswap_create_attr.attr,
> +       &xswap_destroy_attr.attr,
>         NULL,
>  };
>
> @@ -3399,65 +3427,44 @@ static void flush_percpu_swap_cluster(struct swap_info_struct *si)
>  }
>
>
> -SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)

Here you relocate the swapoff function and causing a lot of diff
context due to the code movement.

I move your swapoff code back, right after
flush_percpu_swap_cluster(). Then the diff is much cleaner around
swapoff.

For example, here is what the new diff showed:
SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 {
        struct swap_info_struct *p = NULL;
@@ -3391,6 +3393,11 @@ SYSCALL_DEFINE1(swapoff, const char __us
        spin_unlock(&p->lock);
        spin_unlock(&swap_lock);

+#ifdef CONFIG_XSWAP
+       if (p->flags & SWP_XSWAP)
+               cancel_work_sync(&p->xswap_shrink_work);
+#endif
+
        wait_for_allocation(p);

        set_current_oom_origin();

It means we will need some swap_ops->swapoff() for handling the async
workqueue canceling.

Chris

> +/*
> + * Drop @p from the avail and active lists and undo its accounting.  The
> + * caller must hold swap_lock and have checked that @p is WRITEOK and not
> + * pinned for hibernation.
> + *
> + * Returns 0, or -ENOMEM with swap_lock still held.
> + */
> +static int swap_info_remove(struct swap_info_struct *p)
>  {
> -       struct swap_info_struct *p = NULL;
> -       struct file *swap_file, *victim;
> -       struct address_space *mapping;
> -       struct inode *inode;
> -       int err, found = 0;
> -
> -       if (!capable(CAP_SYS_ADMIN))
> -               return -EPERM;
> -
> -       BUG_ON(!current->mm);
> -
> -       CLASS(filename, pathname)(specialfile);
> -       victim = file_open_name(pathname, O_RDWR|O_LARGEFILE, 0);
> -       if (IS_ERR(victim))
> -               return PTR_ERR(victim);
> -
> -       mapping = victim->f_mapping;
> -       spin_lock(&swap_lock);
> -       plist_for_each_entry(p, &swap_active_head, list) {
> -               if (p->flags & SWP_WRITEOK) {
> -                       if (p->swap_file->f_mapping == mapping) {
> -                               found = 1;
> -                               break;
> -                       }
> -               }
> -       }
> -       if (!found) {
> -               err = -EINVAL;
> -               spin_unlock(&swap_lock);
> -               goto out_dput;
> -       }
> -
> -       /* Refuse swapoff while the device is pinned for hibernation */
> -       if (p->flags & SWP_HIBERNATION) {
> -               err = -EBUSY;
> -               spin_unlock(&swap_lock);
> -               goto out_dput;
> -       }
> -
>         if (!security_vm_enough_memory_mm(current->mm, p->pages))
>                 vm_unacct_memory(p->pages);
> -       else {
> -               err = -ENOMEM;
> -               spin_unlock(&swap_lock);
> -               goto out_dput;
> -       }
> +       else
> +               return -ENOMEM;
> +
>         spin_lock(&p->lock);
>         del_from_avail_list(p, true);
>         plist_del(&p->list, &swap_active_head);
>         atomic_long_sub(p->pages, &nr_swap_pages);
>         total_swap_pages -= p->pages;
>         spin_unlock(&p->lock);
> -       spin_unlock(&swap_lock);
> +       return 0;
> +}
> +
> +/* Common swap teardown after list removal; shared by sys_swapoff() and
> + * xswap_destroy().
> + */
> +static int __swapoff(struct swap_info_struct *p)
> +{
> +       struct file *swap_file = NULL;
> +       int err;
>
>  #ifdef CONFIG_XSWAP
> -       if (p->flags & SWP_XSWAP)
> +       if (p->flags & SWP_XSWAP) {
>                 cancel_work_sync(&p->xswap_shrink_work);
> +               /* Wait out a shrink racing us from the sysfs write path. */
> +               mutex_lock(&p->xswap_lock);
> +               mutex_unlock(&p->xswap_lock);
> +       }
>  #endif
>
>         wait_for_allocation(p);
> @@ -3469,7 +3476,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>         if (err) {
>                 /* re-insert swap space back into swap_list */
>                 reinsert_swap_info(p);
> -               goto out_dput;
> +               return err;
>         }
>
>         /*
> @@ -3509,15 +3516,21 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>         kfree(p->global_cluster);
>         p->global_cluster = NULL;
>         free_swap_cluster_info(p);
> +       /*
> +        * The device is off swap_active_head and no longer WRITEOK, so no
> +        * reader can observe these; clearing them here needs no lock.
> +        */
>         p->max = 0;
>         p->cluster_info = NULL;
>
> -       inode = mapping->host;
> +       if (swap_file) {
> +               struct inode *inode = swap_file->f_mapping->host;
>
> -       inode_lock(inode);
> -       inode->i_flags &= ~S_SWAPFILE;
> -       inode_unlock(inode);
> -       filp_close(swap_file, NULL);
> +               inode_lock(inode);
> +               inode->i_flags &= ~S_SWAPFILE;
> +               inode_unlock(inode);
> +               filp_close(swap_file, NULL);
> +       }
>
>         /*
>          * Clear the SWP_USED flag after all resources are freed so that swapon
> @@ -3528,10 +3541,61 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
>         p->flags = 0;
>         spin_unlock(&swap_lock);
>
> -       err = 0;
>         atomic_inc(&proc_poll_event);
>         wake_up_interruptible(&proc_poll_wait);
>
> +       return 0;
> +}
> +
> +SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
> +{
> +       struct swap_info_struct *p = NULL;
> +       struct file *victim;
> +       struct address_space *mapping;
> +       int err, found = 0;
> +
> +       if (!capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       BUG_ON(!current->mm);
> +
> +       CLASS(filename, pathname)(specialfile);
> +       victim = file_open_name(pathname, O_RDWR|O_LARGEFILE, 0);
> +       if (IS_ERR(victim))
> +               return PTR_ERR(victim);
> +
> +       mapping = victim->f_mapping;
> +       spin_lock(&swap_lock);
> +       plist_for_each_entry(p, &swap_active_head, list) {
> +               if (p->flags & SWP_WRITEOK) {
> +                       if (p->swap_file && p->swap_file->f_mapping == mapping) {
> +                               found = 1;
> +                               break;
> +                       }
> +               }
> +       }
> +       if (!found) {
> +               err = -EINVAL;
> +               spin_unlock(&swap_lock);
> +               goto out_dput;
> +       }
> +
> +       /* Refuse swapoff while the device is pinned for hibernation */
> +       if (p->flags & SWP_HIBERNATION) {
> +               err = -EBUSY;
> +               spin_unlock(&swap_lock);
> +               goto out_dput;
> +       }
> +
> +       err = swap_info_remove(p);
> +       if (err) {
> +               spin_unlock(&swap_lock);
> +               goto out_dput;
> +       }
> +       spin_unlock(&swap_lock);
> +
> +       err = __swapoff(p);
> +
>  out_dput:
>         filp_close(victim, NULL);
>         return err;
> @@ -4128,6 +4192,10 @@ static void xswap_try_shrink(struct swap_info_struct *si)
>
>         mutex_lock(&si->xswap_lock);
>
> +       /* A swapoff raced us and is about to walk this mapping. */
> +       if (!(READ_ONCE(si->flags) & SWP_WRITEOK))
> +               goto out_unlock;
> +
>         nr_mapped = READ_ONCE(si->nr_clusters_mapped);
>         if (nr_mapped <= 1)     /* keep cluster 0 */
>                 goto out_unlock;
> @@ -4435,6 +4503,35 @@ static int xswap_create(int prio)
>         spin_unlock(&swap_lock);
>         return error;
>  }
> +
> +/* Tear down a file-less xswap device by its swap type. */
> +static int xswap_destroy(int type)
> +{
> +       struct swap_info_struct *p;
> +       int err;
> +
> +       p = swap_type_to_info(type);
> +       if (!p)
> +               return -EINVAL;
> +
> +       spin_lock(&swap_lock);
> +       if (!(p->flags & SWP_WRITEOK) || !(p->flags & SWP_XSWAP)) {
> +               spin_unlock(&swap_lock);
> +               return -EINVAL;
> +       }
> +       /* Refuse swapoff while the device is pinned for hibernation */
> +       if (p->flags & SWP_HIBERNATION) {
> +               spin_unlock(&swap_lock);
> +               return -EBUSY;
> +       }
> +
> +       err = swap_info_remove(p);
> +       spin_unlock(&swap_lock);
> +       if (err)
> +               return err;
> +
> +       return __swapoff(p);
> +}
>  #endif /* CONFIG_XSWAP */
>
>  SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
> --
> 2.54.0
>