From: Baoquan He <bhe@redhat.com>
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.
Suggested-by: Chris Li <chrisl@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
Co-developed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Barry Song <baohua@kernel.org>
---
include/linux/swap.h | 2 +
mm/swap.h | 10 ++++-
mm/swap_io.c | 99 +++++++++++++++++++++++++-------------------
mm/swapfile.c | 1 +
mm/zswap.c | 3 +-
5 files changed, 70 insertions(+), 45 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 1930f81e6be4..b52f30dad72b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -243,6 +243,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.
*/
@@ -283,6 +284,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 161185057993..219dbcb3ffb1 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -217,6 +217,15 @@ extern void __swap_cluster_free_entries(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);
+};
+void setup_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)
@@ -226,7 +235,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 91b33d955e63..ad0895af6ed8 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;
@@ -283,7 +284,8 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
}
rcu_read_unlock();
- __swap_writepage(folio, swap_plug);
+ VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
+ sis->ops->write_folio(sis, folio, swap_plug);
return 0;
out_unlock:
folio_unlock(folio);
@@ -373,10 +375,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);
@@ -409,8 +412,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;
@@ -429,8 +433,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;
@@ -446,29 +451,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,
- * 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,
- * 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;
@@ -537,9 +519,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);
@@ -571,8 +554,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;
@@ -593,8 +577,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;
@@ -608,6 +593,39 @@ 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,
+};
+
+void setup_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;
+}
+
void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
{
struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
@@ -642,13 +660,8 @@ 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);
- }
+ VM_WARN_ON_ONCE(!sis->ops || !sis->ops->read_folio);
+ sis->ops->read_folio(sis, folio, plug);
finish:
if (workingset) {
diff --git a/mm/swapfile.c b/mm/swapfile.c
index ff315b752afd..c9eb95141c8f 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3732,6 +3732,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
si->list.prio = -si->prio;
si->avail_list.prio = -si->prio;
si->swap_file = swap_file;
+ setup_swap_ops(si);
/* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
enable_swap_info(si);
diff --git a/mm/zswap.c b/mm/zswap.c
index 4b5149173b0e..9bacb1733e1c 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1054,7 +1054,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
folio_set_reclaim(folio);
/* start writeback */
- __swap_writepage(folio, NULL);
+ VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
+ si->ops->write_folio(si, folio, NULL);
out:
if (ret && ret != -EEXIST) {
--
2.39.3 (Apple Git-146)On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
>
> From: Baoquan He <bhe@redhat.com>
>
> 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.
Should add that there is no functional change expected from this patch.
> Suggested-by: Chris Li <chrisl@kernel.org>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Co-developed-by: Barry Song <baohua@kernel.org>
> Signed-off-by: Barry Song <baohua@kernel.org>
Have some nitpicks follows.
> ---
> include/linux/swap.h | 2 +
> mm/swap.h | 10 ++++-
> mm/swap_io.c | 99 +++++++++++++++++++++++++-------------------
> mm/swapfile.c | 1 +
> mm/zswap.c | 3 +-
> 5 files changed, 70 insertions(+), 45 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 1930f81e6be4..b52f30dad72b 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -243,6 +243,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.
> */
> @@ -283,6 +284,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 161185057993..219dbcb3ffb1 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -217,6 +217,15 @@ extern void __swap_cluster_free_entries(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);
> +};
> +void setup_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)
> @@ -226,7 +235,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 91b33d955e63..ad0895af6ed8 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;
> @@ -283,7 +284,8 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
> }
> rcu_read_unlock();
>
> - __swap_writepage(folio, swap_plug);
> + VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
> + sis->ops->write_folio(sis, folio, swap_plug);
> return 0;
> out_unlock:
> folio_unlock(folio);
> @@ -373,10 +375,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);
>
> @@ -409,8 +412,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;
> @@ -429,8 +433,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;
>
> @@ -446,29 +451,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,
> - * 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,
> - * 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;
> @@ -537,9 +519,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);
>
> @@ -571,8 +554,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;
> @@ -593,8 +577,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;
>
> @@ -608,6 +593,39 @@ 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,
> +};
> +
> +void setup_swap_ops(struct swap_info_struct *sis)
setup_swap_ops()` needs to return an indication of an error.
The error is that sis->ops is NULL or op->read_folio == NULL or
op->write_folio == NULL.
If there is an error, we should fail the swapon.
See my other comments regardin g the error checking at the dereference side.
> +{
> + /*
> + * ->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;
> +}
> +
> void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
> {
> struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
> @@ -642,13 +660,8 @@ 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);
> - }
> + VM_WARN_ON_ONCE(!sis->ops || !sis->ops->read_folio);
WARN_ON_ONCE is not enough. If we hit a WARN here, the kernel will
crash on the next dereference of ops->read_folios().
In other words, we will get kernel OOPS immediately after the warning.
This makes no make sense.
We should refuse to swapon at setup_swap_ops() above. Then no
checking here is needed.
> + sis->ops->read_folio(sis, folio, plug);
>
> finish:
> if (workingset) {
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index ff315b752afd..c9eb95141c8f 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3732,6 +3732,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
> si->list.prio = -si->prio;
> si->avail_list.prio = -si->prio;
> si->swap_file = swap_file;
> + setup_swap_ops(si);
>
> /* Sets SWP_WRITEOK, resurrect the percpu ref, expose the swap device */
> enable_swap_info(si);
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 4b5149173b0e..9bacb1733e1c 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1054,7 +1054,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> folio_set_reclaim(folio);
>
> /* start writeback */
> - __swap_writepage(folio, NULL);
> + VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
Again, WARN_ON is not enough, the kernel will panic here if a warning
is triggered.
Check at setup_swap_ops() and refuse to swapon if ops->write_folio is NULL.
With the other "sis" fix, the patch looks good otherwise.
Chris
> + si->ops->write_folio(si, folio, NULL);
>
> out:
> if (ret && ret != -EEXIST) {
> --
> 2.39.3 (Apple Git-146)
>
On Tue, Mar 31, 2026 at 4:31 PM Chris Li <chrisl@kernel.org> wrote:
>
> On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
> >
> > From: Baoquan He <bhe@redhat.com>
> >
> > 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.
>
> Should add that there is no functional change expected from this patch.
Sounds good to me.
[...]
> > +static const struct swap_ops bdev_async_swap_ops = {
> > + .read_folio = swap_read_folio_bdev_async,
> > + .write_folio = swap_writepage_bdev_async,
> > +};
> > +
> > +void setup_swap_ops(struct swap_info_struct *sis)
>
> setup_swap_ops()` needs to return an indication of an error.
> The error is that sis->ops is NULL or op->read_folio == NULL or
> op->write_folio == NULL.
> If there is an error, we should fail the swapon.
Right now there is no possibility for any of the ops (read_folio or
write_folio) to be NULL, so I was using VM_WARN_ON as an assertion
to catch bugs in kernel code.
That said, I agree we can return an error earlier instead of letting
swapon fail later.
I think it may still make sense to keep a VM_WARN_ON in
setup_swap_ops() before returning -EINVAL or a similar error?
Thanks
Barry
On Tue, Mar 31, 2026 at 2:22 AM Barry Song <baohua@kernel.org> wrote:
>
> On Tue, Mar 31, 2026 at 4:31 PM Chris Li <chrisl@kernel.org> wrote:
> >
> > On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
> > >
> > > From: Baoquan He <bhe@redhat.com>
> > >
> > > 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.
> >
> > Should add that there is no functional change expected from this patch.
>
> Sounds good to me.
Ack
>
> > > +static const struct swap_ops bdev_async_swap_ops = {
> > > + .read_folio = swap_read_folio_bdev_async,
> > > + .write_folio = swap_writepage_bdev_async,
> > > +};
> > > +
> > > +void setup_swap_ops(struct swap_info_struct *sis)
> >
> > setup_swap_ops()` needs to return an indication of an error.
> > The error is that sis->ops is NULL or op->read_folio == NULL or
> > op->write_folio == NULL.
> > If there is an error, we should fail the swapon.
>
> Right now there is no possibility for any of the ops (read_folio or
> write_folio) to be NULL, so I was using VM_WARN_ON as an assertion
> to catch bugs in kernel code.
Ack. Someone might register a new swap_ops later that introduces a
NULL swap_write_folio. The check is for catching that.
>
> That said, I agree we can return an error earlier instead of letting
> swapon fail later.
>
> I think it may still make sense to keep a VM_WARN_ON in
> setup_swap_ops() before returning -EINVAL or a similar error?
Sure. We can just print pr_err() rather than VM_WARN_ON. VM_WARN_ON
is nop when VM debug is disabled. It is a kernel swap code bug on the
swapon path, so the back trace portion of the warning is not very
valuable. A normal error print will suffice. Because the swapon
failed, the user will notice this error.
Chris
Hi Barry,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Barry-Song/mm-swap-rename-mm-page_io-c-to-mm-swap_io-c/20260328-170852
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20260328075812.11060-3-21cnbao%40gmail.com
patch subject: [PATCH v2 2/3] mm/swap: use swap_ops to register swap device's methods
config: um-randconfig-r072-20260329 (https://download.01.org/0day-ci/archive/20260329/202603291919.Y8MwFsok-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
smatch: v0.5.0-9004-gb810ac53
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603291919.Y8MwFsok-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603291919.Y8MwFsok-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/asm-generic/bug.h:7,
from ./arch/um/include/generated/asm/bug.h:1,
from arch/x86/include/asm/alternative.h:9,
from arch/x86/um/asm/barrier.h:6,
from include/linux/list.h:11,
from include/linux/module.h:12,
from mm/zswap.c:16:
mm/zswap.c: In function 'zswap_writeback_entry':
>> mm/zswap.c:1057:26: error: 'sis' undeclared (first use in this function); did you mean 'si'?
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~
include/linux/once_lite.h:28:41: note: in definition of macro 'DO_ONCE_LITE_IF'
28 | bool __ret_do_once = !!(condition); \
| ^~~~~~~~~
include/linux/mmdebug.h:123:37: note: in expansion of macro 'WARN_ON_ONCE'
123 | #define VM_WARN_ON_ONCE(cond) (void)WARN_ON_ONCE(cond)
| ^~~~~~~~~~~~
mm/zswap.c:1057:9: note: in expansion of macro 'VM_WARN_ON_ONCE'
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~~~~~~~~~~~~~
mm/zswap.c:1057:26: note: each undeclared identifier is reported only once for each function it appears in
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~
include/linux/once_lite.h:28:41: note: in definition of macro 'DO_ONCE_LITE_IF'
28 | bool __ret_do_once = !!(condition); \
| ^~~~~~~~~
include/linux/mmdebug.h:123:37: note: in expansion of macro 'WARN_ON_ONCE'
123 | #define VM_WARN_ON_ONCE(cond) (void)WARN_ON_ONCE(cond)
| ^~~~~~~~~~~~
mm/zswap.c:1057:9: note: in expansion of macro 'VM_WARN_ON_ONCE'
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~~~~~~~~~~~~~
vim +1057 mm/zswap.c
971
972 /*********************************
973 * writeback code
974 **********************************/
975 /*
976 * Attempts to free an entry by adding a folio to the swap cache,
977 * decompressing the entry data into the folio, and issuing a
978 * bio write to write the folio back to the swap device.
979 *
980 * This can be thought of as a "resumed writeback" of the folio
981 * to the swap device. We are basically resuming the same swap
982 * writeback path that was intercepted with the zswap_store()
983 * in the first place. After the folio has been decompressed into
984 * the swap cache, the compressed version stored by zswap can be
985 * freed.
986 */
987 static int zswap_writeback_entry(struct zswap_entry *entry,
988 swp_entry_t swpentry)
989 {
990 struct xarray *tree;
991 pgoff_t offset = swp_offset(swpentry);
992 struct folio *folio;
993 struct mempolicy *mpol;
994 bool folio_was_allocated;
995 struct swap_info_struct *si;
996 int ret = 0;
997
998 /* try to allocate swap cache folio */
999 si = get_swap_device(swpentry);
1000 if (!si)
1001 return -EEXIST;
1002
1003 mpol = get_task_policy(current);
1004 folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, mpol,
1005 NO_INTERLEAVE_INDEX, &folio_was_allocated);
1006 put_swap_device(si);
1007 if (!folio)
1008 return -ENOMEM;
1009
1010 /*
1011 * Found an existing folio, we raced with swapin or concurrent
1012 * shrinker. We generally writeback cold folios from zswap, and
1013 * swapin means the folio just became hot, so skip this folio.
1014 * For unlikely concurrent shrinker case, it will be unlinked
1015 * and freed when invalidated by the concurrent shrinker anyway.
1016 */
1017 if (!folio_was_allocated) {
1018 ret = -EEXIST;
1019 goto out;
1020 }
1021
1022 /*
1023 * folio is locked, and the swapcache is now secured against
1024 * concurrent swapping to and from the slot, and concurrent
1025 * swapoff so we can safely dereference the zswap tree here.
1026 * Verify that the swap entry hasn't been invalidated and recycled
1027 * behind our backs, to avoid overwriting a new swap folio with
1028 * old compressed data. Only when this is successful can the entry
1029 * be dereferenced.
1030 */
1031 tree = swap_zswap_tree(swpentry);
1032 if (entry != xa_load(tree, offset)) {
1033 ret = -ENOMEM;
1034 goto out;
1035 }
1036
1037 if (!zswap_decompress(entry, folio)) {
1038 ret = -EIO;
1039 goto out;
1040 }
1041
1042 xa_erase(tree, offset);
1043
1044 count_vm_event(ZSWPWB);
1045 if (entry->objcg)
1046 count_objcg_events(entry->objcg, ZSWPWB, 1);
1047
1048 zswap_entry_free(entry);
1049
1050 /* folio is up to date */
1051 folio_mark_uptodate(folio);
1052
1053 /* move it to the tail of the inactive list after end_writeback */
1054 folio_set_reclaim(folio);
1055
1056 /* start writeback */
> 1057 VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
1058 si->ops->write_folio(si, folio, NULL);
1059
1060 out:
1061 if (ret && ret != -EEXIST) {
1062 swap_cache_del_folio(folio);
1063 folio_unlock(folio);
1064 }
1065 folio_put(folio);
1066 return ret;
1067 }
1068
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Barry,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Barry-Song/mm-swap-rename-mm-page_io-c-to-mm-swap_io-c/20260328-170852
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20260328075812.11060-3-21cnbao%40gmail.com
patch subject: [PATCH v2 2/3] mm/swap: use swap_ops to register swap device's methods
config: arm-randconfig-001-20260329 (https://download.01.org/0day-ci/archive/20260329/202603291844.TWh2Ellp-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 054e11d1a17e5ba88bb1a8ef32fad3346e80b186)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603291844.TWh2Ellp-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603291844.TWh2Ellp-lkp@intel.com/
All errors (new ones prefixed by >>):
>> mm/zswap.c:1057:19: error: use of undeclared identifier 'sis'; did you mean 'si'?
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~
| si
include/linux/mmdebug.h:133:52: note: expanded from macro 'VM_WARN_ON_ONCE'
133 | #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
| ^~~~
include/linux/build_bug.h:30:63: note: expanded from macro 'BUILD_BUG_ON_INVALID'
30 | #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
| ^
mm/zswap.c:995:27: note: 'si' declared here
995 | struct swap_info_struct *si;
| ^
mm/zswap.c:1057:32: error: use of undeclared identifier 'sis'; did you mean 'si'?
1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
| ^~~
| si
include/linux/mmdebug.h:133:52: note: expanded from macro 'VM_WARN_ON_ONCE'
133 | #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond)
| ^~~~
include/linux/build_bug.h:30:63: note: expanded from macro 'BUILD_BUG_ON_INVALID'
30 | #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
| ^
mm/zswap.c:995:27: note: 'si' declared here
995 | struct swap_info_struct *si;
| ^
2 errors generated.
vim +1057 mm/zswap.c
971
972 /*********************************
973 * writeback code
974 **********************************/
975 /*
976 * Attempts to free an entry by adding a folio to the swap cache,
977 * decompressing the entry data into the folio, and issuing a
978 * bio write to write the folio back to the swap device.
979 *
980 * This can be thought of as a "resumed writeback" of the folio
981 * to the swap device. We are basically resuming the same swap
982 * writeback path that was intercepted with the zswap_store()
983 * in the first place. After the folio has been decompressed into
984 * the swap cache, the compressed version stored by zswap can be
985 * freed.
986 */
987 static int zswap_writeback_entry(struct zswap_entry *entry,
988 swp_entry_t swpentry)
989 {
990 struct xarray *tree;
991 pgoff_t offset = swp_offset(swpentry);
992 struct folio *folio;
993 struct mempolicy *mpol;
994 bool folio_was_allocated;
995 struct swap_info_struct *si;
996 int ret = 0;
997
998 /* try to allocate swap cache folio */
999 si = get_swap_device(swpentry);
1000 if (!si)
1001 return -EEXIST;
1002
1003 mpol = get_task_policy(current);
1004 folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, mpol,
1005 NO_INTERLEAVE_INDEX, &folio_was_allocated);
1006 put_swap_device(si);
1007 if (!folio)
1008 return -ENOMEM;
1009
1010 /*
1011 * Found an existing folio, we raced with swapin or concurrent
1012 * shrinker. We generally writeback cold folios from zswap, and
1013 * swapin means the folio just became hot, so skip this folio.
1014 * For unlikely concurrent shrinker case, it will be unlinked
1015 * and freed when invalidated by the concurrent shrinker anyway.
1016 */
1017 if (!folio_was_allocated) {
1018 ret = -EEXIST;
1019 goto out;
1020 }
1021
1022 /*
1023 * folio is locked, and the swapcache is now secured against
1024 * concurrent swapping to and from the slot, and concurrent
1025 * swapoff so we can safely dereference the zswap tree here.
1026 * Verify that the swap entry hasn't been invalidated and recycled
1027 * behind our backs, to avoid overwriting a new swap folio with
1028 * old compressed data. Only when this is successful can the entry
1029 * be dereferenced.
1030 */
1031 tree = swap_zswap_tree(swpentry);
1032 if (entry != xa_load(tree, offset)) {
1033 ret = -ENOMEM;
1034 goto out;
1035 }
1036
1037 if (!zswap_decompress(entry, folio)) {
1038 ret = -EIO;
1039 goto out;
1040 }
1041
1042 xa_erase(tree, offset);
1043
1044 count_vm_event(ZSWPWB);
1045 if (entry->objcg)
1046 count_objcg_events(entry->objcg, ZSWPWB, 1);
1047
1048 zswap_entry_free(entry);
1049
1050 /* folio is up to date */
1051 folio_mark_uptodate(folio);
1052
1053 /* move it to the tail of the inactive list after end_writeback */
1054 folio_set_reclaim(folio);
1055
1056 /* start writeback */
> 1057 VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio);
1058 si->ops->write_folio(si, folio, NULL);
1059
1060 out:
1061 if (ret && ret != -EEXIST) {
1062 swap_cache_del_folio(folio);
1063 folio_unlock(folio);
1064 }
1065 folio_put(folio);
1066 return ret;
1067 }
1068
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Sun, Mar 29, 2026 at 6:49 PM kernel test robot <lkp@intel.com> wrote: > > Hi Barry, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on akpm-mm/mm-everything] > > url: https://github.com/intel-lab-lkp/linux/commits/Barry-Song/mm-swap-rename-mm-page_io-c-to-mm-swap_io-c/20260328-170852 > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > patch link: https://lore.kernel.org/r/20260328075812.11060-3-21cnbao%40gmail.com > patch subject: [PATCH v2 2/3] mm/swap: use swap_ops to register swap device's methods > config: arm-randconfig-001-20260329 (https://download.01.org/0day-ci/archive/20260329/202603291844.TWh2Ellp-lkp@intel.com/config) > compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 054e11d1a17e5ba88bb1a8ef32fad3346e80b186) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603291844.TWh2Ellp-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202603291844.TWh2Ellp-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > >> mm/zswap.c:1057:19: error: use of undeclared identifier 'sis'; did you mean 'si'? > 1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio); > | ^~~ > | si > include/linux/mmdebug.h:133:52: note: expanded from macro 'VM_WARN_ON_ONCE' > 133 | #define VM_WARN_ON_ONCE(cond) BUILD_BUG_ON_INVALID(cond) > | ^~~~ > include/linux/build_bug.h:30:63: note: expanded from macro 'BUILD_BUG_ON_INVALID' > 30 | #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e)))) > | ^ > mm/zswap.c:995:27: note: 'si' declared here > 995 | struct swap_info_struct *si; > | ^ > mm/zswap.c:1057:32: error: use of undeclared identifier 'sis'; did you mean 'si'? > 1057 | VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio); > | ^~~ > | si Deeply sorry for the last-minute edit—I fat-fingered it. It should be si, not sis. My sincere apologies. diff --git a/mm/zswap.c b/mm/zswap.c index 9bacb1733e1c..b60821758169 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1054,7 +1054,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry, folio_set_reclaim(folio); /* start writeback */ - VM_WARN_ON_ONCE(!sis->ops || !sis->ops->write_folio); + VM_WARN_ON_ONCE(!si->ops || !si->ops->write_folio); si->ops->write_folio(si, folio, NULL); out: -- Best Regards Barry
© 2016 - 2026 Red Hat, Inc.