Re: [PATCH v3 2/3] mm/swap: use swap_ops to register swap device's methods

Usama Arif
2026-04-15T15:05:59Z

On Wed, 15 Apr 2026 16:56:57 +0800 Baoquan He <baoquan.he@linux.dev> wrote:

> This simplifies codes and makes logic clearer. And also makes later any
> new swap device type being added easier to handle.
>
> Currently there are three types of swap devices: bdev_fs, bdev_sync
> and bdev_async, and only operations read_folio and write_folio are
> included. In the future, there could be more swap device types added
> and more appropriate opeations adapted into swap_ops.

s/opeations/operations/

>
> Suggested-by: Chris Li <chrisl@kernel.org>
> Co-developed-by: Barry Song <baohua@kernel.org>
> Signed-off-by: Barry Song <baohua@kernel.org>
> Signed-off-by: Baoquan He <baoquan.he@linux.dev>
> ---
> include/linux/swap.h | 2 +
> mm/swap.h | 10 ++++-
> mm/swap_io.c | 102 +++++++++++++++++++++++++------------------
> mm/swapfile.c | 5 +++
> mm/zswap.c | 2 +-
> 5 files changed, 76 insertions(+), 45 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 62fc7499b408..3069bbfd7a05 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -255,6 +255,7 @@ struct swap_sequential_cluster {
> unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offset */
> };
>
> +struct swap_ops;
> /*
> * The in-memory structure used to track swap areas.
> */
> @@ -300,6 +301,7 @@ struct swap_info_struct {
> struct work_struct reclaim_work; /* reclaim worker */
> struct list_head discard_clusters; /* discard clusters list */
> struct plist_node avail_list; /* entry in swap_avail_head */
> + const struct swap_ops *ops;
> };
>
> static inline swp_entry_t page_swap_entry(struct page *page)
> diff --git a/mm/swap.h b/mm/swap.h
> index 81a80605fc6a..0f4c597e3f28 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -213,6 +213,15 @@ extern void swap_entries_free(struct swap_info_struct *si,
> /* linux/mm/swap_io.c */
> int sio_pool_init(void);
> struct swap_iocb;
> +struct swap_ops {
> + void (*read_folio)(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug);
> + void (*write_folio)(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug);
> +};
> +int init_swap_ops(struct swap_info_struct *sis);
> void swap_read_folio(struct folio *folio, struct swap_iocb **plug);
> void __swap_read_unplug(struct swap_iocb *plug);
> static inline void swap_read_unplug(struct swap_iocb *plug)
> @@ -222,7 +231,6 @@ static inline void swap_read_unplug(struct swap_iocb *plug)
> }
> void swap_write_unplug(struct swap_iocb *sio);
> int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug);
> -void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug);
>
> /* linux/mm/swap_state.c */
> extern struct address_space swap_space __read_mostly;
> diff --git a/mm/swap_io.c b/mm/swap_io.c
> index 4bf210bd677e..602e9b871b30 100644
> --- a/mm/swap_io.c
> +++ b/mm/swap_io.c
> @@ -238,6 +238,7 @@ static void swap_zeromap_folio_clear(struct folio *folio)
> int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
> {
> int ret = 0;
> + struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
>
> if (folio_free_swap(folio))
> goto out_unlock;
> @@ -279,7 +280,7 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
> return AOP_WRITEPAGE_ACTIVATE;
> }
>
> - __swap_writepage(folio, swap_plug);
> + sis->ops->write_folio(sis, folio, swap_plug);
> return 0;
> out_unlock:
> folio_unlock(folio);
> @@ -369,10 +370,11 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
> mempool_free(sio, sio_pool);
> }
>
> -static void swap_writepage_fs(struct folio *folio, struct swap_iocb **swap_plug)
> +static void swap_writepage_fs(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **swap_plug)
> {
> struct swap_iocb *sio = swap_plug ? *swap_plug : NULL;
> - struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> struct file *swap_file = sis->swap_file;
> loff_t pos = swap_dev_pos(folio->swap);
>
> @@ -405,8 +407,9 @@ static void swap_writepage_fs(struct folio *folio, struct swap_iocb **swap_plug)
> *swap_plug = sio;
> }
>
> -static void swap_writepage_bdev_sync(struct folio *folio,
> - struct swap_info_struct *sis)
> +static void swap_writepage_bdev_sync(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug)
> {
> struct bio_vec bv;
> struct bio bio;
> @@ -425,8 +428,9 @@ static void swap_writepage_bdev_sync(struct folio *folio,
> __end_swap_bio_write(&bio);
> }
>
> -static void swap_writepage_bdev_async(struct folio *folio,
> - struct swap_info_struct *sis)
> +static void swap_writepage_bdev_async(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug)
> {
> struct bio *bio;
>
> @@ -442,29 +446,6 @@ static void swap_writepage_bdev_async(struct folio *folio,
> submit_bio(bio);
> }
>
> -void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug)
> -{
> - struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> -
> - VM_BUG_ON_FOLIO(!folio_test_swapcache(folio), folio);
> - /*
> - * ->flags can be updated non-atomically (scan_swap_map_slots),
> - * but that will never affect SWP_FS_OPS, so the data_race
> - * is safe.
> - */
> - if (data_race(sis->flags & SWP_FS_OPS))
> - swap_writepage_fs(folio, swap_plug);
> - /*
> - * ->flags can be updated non-atomically (scan_swap_map_slots),
> - * but that will never affect SWP_SYNCHRONOUS_IO, so the data_race
> - * is safe.
> - */
> - else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> - swap_writepage_bdev_sync(folio, sis);
> - else
> - swap_writepage_bdev_async(folio, sis);
> -}
> -
> void swap_write_unplug(struct swap_iocb *sio)
> {
> struct iov_iter from;
> @@ -533,9 +514,10 @@ static bool swap_read_folio_zeromap(struct folio *folio)
> return true;
> }
>
> -static void swap_read_folio_fs(struct folio *folio, struct swap_iocb **plug)
> +static void swap_read_folio_fs(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug)
> {
> - struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> struct swap_iocb *sio = NULL;
> loff_t pos = swap_dev_pos(folio->swap);
>
> @@ -567,8 +549,9 @@ static void swap_read_folio_fs(struct folio *folio, struct swap_iocb **plug)
> *plug = sio;
> }
>
> -static void swap_read_folio_bdev_sync(struct folio *folio,
> - struct swap_info_struct *sis)
> +static void swap_read_folio_bdev_sync(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug)
> {
> struct bio_vec bv;
> struct bio bio;
> @@ -589,8 +572,9 @@ static void swap_read_folio_bdev_sync(struct folio *folio,
> put_task_struct(current);
> }
>
> -static void swap_read_folio_bdev_async(struct folio *folio,
> - struct swap_info_struct *sis)
> +static void swap_read_folio_bdev_async(struct swap_info_struct *sis,
> + struct folio *folio,
> + struct swap_iocb **plug)
> {
> struct bio *bio;
>
> @@ -604,6 +588,44 @@ static void swap_read_folio_bdev_async(struct folio *folio,
> submit_bio(bio);
> }
>
> +static const struct swap_ops bdev_fs_swap_ops = {
> + .read_folio = swap_read_folio_fs,
> + .write_folio = swap_writepage_fs,
> +};
> +
> +static const struct swap_ops bdev_sync_swap_ops = {
> + .read_folio = swap_read_folio_bdev_sync,
> + .write_folio = swap_writepage_bdev_sync,
> +};
> +
> +static const struct swap_ops bdev_async_swap_ops = {
> + .read_folio = swap_read_folio_bdev_async,
> + .write_folio = swap_writepage_bdev_async,
> +};
> +
> +int init_swap_ops(struct swap_info_struct *sis)
> +{
> + /*
> + * ->flags can be updated non-atomically, but that will
> + * never affect SWP_FS_OPS, so the data_race is safe.
> + */
> + if (data_race(sis->flags & SWP_FS_OPS))
> + sis->ops = &bdev_fs_swap_ops;
> + /*
> + * ->flags can be updated non-atomically, but that will
> + * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
> + */
> + else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> + sis->ops = &bdev_sync_swap_ops;
> + else
> + sis->ops = &bdev_async_swap_ops;
> +
> + if (sis->ops || sis->ops->read_folio || sis->ops->write_folio)
> + return -1;

sis->ops is always non-NULL, and you will return -1 here and
swapon will always fail with -EINVAL.

You probably wanted?

if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
return -1;

> +
> + return 0;
> +}
> +
> void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
> {
> struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> @@ -638,13 +660,7 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
> /* We have to read from slower devices. Increase zswap protection. */
> zswap_folio_swapin(folio);
>
> - if (data_race(sis->flags & SWP_FS_OPS)) {
> - swap_read_folio_fs(folio, plug);
> - } else if (synchronous) {
> - swap_read_folio_bdev_sync(folio, sis);
> - } else {
> - swap_read_folio_bdev_async(folio, sis);
> - }
> + sis->ops->read_folio(sis, folio, plug);
>
> finish:
> if (workingset) {
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 60e21414624b..7e1660f59c00 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3413,6 +3413,11 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
> goto bad_swap_unlock_inode;
> }
>
> + if (init_swap_ops(si)) {
> + error = -EINVAL;
> + goto bad_swap_unlock_inode;
> + }
> +
> si->max = maxpages;
> si->pages = maxpages - 1;
> nr_extents = setup_swap_extents(si, &span);
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 16b2ef7223e1..d8930d9054df 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1061,7 +1061,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> folio_set_reclaim(folio);
>
> /* start writeback */
> - __swap_writepage(folio, NULL);
> + si->ops->write_folio(si, folio, NULL);
>
> out:
> if (ret && ret != -EEXIST) {
> --
> 2.52.0
>
>