[PATCH v2 0/3] mm/swap: use swap_ops to register swap device's methods

[PATCH v2 0/3] mm/swap: use swap_ops to register swap device's methods

[Barry Song]

From: Barry Song <baohua@kernel.org>

-v2:
 * lots of cleanup for patch 2/3: renaming, moving data
   structures, and using const properly
 * collected tags from Kairui, Nhat and Barry

-v1:
 https://lore.kernel.org/linux-mm/20260302104016.163542-1-bhe@redhat.com/

This can simplify the code logic and benefit any new type of swap device
added later.

And also do renaming in this patchset:
-------
   file renaming:
   ---
   mm/page_io.c to mm/swap_io.c

   function renaming:
   ---
   swap_writepage_* to swap_write_folio_* in file mm/swap_io.c

Baoquan He (3):
  mm/swap: rename mm/page_io.c to mm/swap_io.c
  mm/swap: use swap_ops to register swap device's methods
  mm/swap_io.c: rename swap_writepage_* to swap_write_folio_*

 MAINTAINERS                 |   2 +-
 include/linux/swap.h        |   2 +
 mm/Makefile                 |   2 +-
 mm/swap.h                   |  12 ++++-
 mm/{page_io.c => swap_io.c} | 101 ++++++++++++++++++++----------------
 mm/swapfile.c               |   1 +
 mm/zswap.c                  |   3 +-
 7 files changed, 73 insertions(+), 50 deletions(-)
 rename mm/{page_io.c => swap_io.c} (89%)

-- 
2.39.3 (Apple Git-146)

[PATCH v2 1/3] mm/swap: rename mm/page_io.c to mm/swap_io.c

[Barry Song]

From: Baoquan He <bhe@redhat.com>

Codes in mm/page_io.c are only related to swap io, it has
nothing to do with other page io.

Rename it to avoid confusion.

Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Reviewed-by: Barry Song <baohua@kernel.org>
Acked-by: Kairui Song <kasong@tencent.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Barry Song <baohua@kernel.org>
---
 MAINTAINERS                 | 2 +-
 mm/Makefile                 | 2 +-
 mm/swap.h                   | 2 +-
 mm/{page_io.c => swap_io.c} | 2 --
 4 files changed, 3 insertions(+), 5 deletions(-)
 rename mm/{page_io.c => swap_io.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 16874c32e288..bb7c1031886d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16915,7 +16915,7 @@ F:	Documentation/mm/swap-table.rst
 F:	include/linux/swap.h
 F:	include/linux/swapfile.h
 F:	include/linux/swapops.h
-F:	mm/page_io.c
+F:	mm/swap_io.c
 F:	mm/swap.c
 F:	mm/swap.h
 F:	mm/swap_table.h
diff --git a/mm/Makefile b/mm/Makefile
index 8ad2ab08244e..a65ac900096a 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -75,7 +75,7 @@ ifdef CONFIG_MMU
 	obj-$(CONFIG_ADVISE_SYSCALLS)	+= madvise.o
 endif
 
-obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
+obj-$(CONFIG_SWAP)	+= swap_io.o swap_state.o swapfile.o
 obj-$(CONFIG_ZSWAP)	+= zswap.o
 obj-$(CONFIG_HAS_DMA)	+= dmapool.o
 obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o hugetlb_sysfs.o hugetlb_sysctl.o
diff --git a/mm/swap.h b/mm/swap.h
index a77016f2423b..161185057993 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -214,7 +214,7 @@ extern void __swap_cluster_free_entries(struct swap_info_struct *si,
 					struct swap_cluster_info *ci,
 					unsigned int ci_off, unsigned int nr_pages);
 
-/* linux/mm/page_io.c */
+/* linux/mm/swap_io.c */
 int sio_pool_init(void);
 struct swap_iocb;
 void swap_read_folio(struct folio *folio, struct swap_iocb **plug);
diff --git a/mm/page_io.c b/mm/swap_io.c
similarity index 99%
rename from mm/page_io.c
rename to mm/swap_io.c
index 70cea9e24d2f..91b33d955e63 100644
--- a/mm/page_io.c
+++ b/mm/swap_io.c
@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- *  linux/mm/page_io.c
- *
  *  Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
  *
  *  Swap reorganised 29.12.95, 
-- 
2.39.3 (Apple Git-146)

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

[Barry Song]

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)

[PATCH v2 3/3] mm/swap_io.c: rename swap_writepage_* to swap_write_folio_*

[Barry Song]

From: Baoquan He <bhe@redhat.com>

All these swap_writepage_* functions are hanlding passed in folio, but
not page. And this renaming make them consistent with the their
counterpart swap_read_folio_* functions.

Reviewed-by: Barry Song <baohua@kernel.org>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Barry Song <baohua@kernel.org>
---
 mm/swap_io.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/swap_io.c b/mm/swap_io.c
index ad0895af6ed8..063cd005ca0d 100644
--- a/mm/swap_io.c
+++ b/mm/swap_io.c
@@ -375,7 +375,7 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
 	mempool_free(sio, sio_pool);
 }
 
-static void swap_writepage_fs(struct swap_info_struct *sis,
+static void swap_write_folio_fs(struct swap_info_struct *sis,
 			      struct folio *folio,
 			      struct swap_iocb **swap_plug)
 {
@@ -412,7 +412,7 @@ static void swap_writepage_fs(struct swap_info_struct *sis,
 		*swap_plug = sio;
 }
 
-static void swap_writepage_bdev_sync(struct swap_info_struct *sis,
+static void swap_write_folio_bdev_sync(struct swap_info_struct *sis,
 				     struct folio *folio,
 				     struct swap_iocb **plug)
 {
@@ -433,7 +433,7 @@ static void swap_writepage_bdev_sync(struct swap_info_struct *sis,
 	__end_swap_bio_write(&bio);
 }
 
-static void swap_writepage_bdev_async(struct swap_info_struct *sis,
+static void swap_write_folio_bdev_async(struct swap_info_struct *sis,
 				      struct folio *folio,
 				      struct swap_iocb **plug)
 {
@@ -595,17 +595,17 @@ static void swap_read_folio_bdev_async(struct swap_info_struct *sis,
 
 static const struct swap_ops bdev_fs_swap_ops = {
 	.read_folio = swap_read_folio_fs,
-	.write_folio = swap_writepage_fs,
+	.write_folio = swap_write_folio_fs,
 };
 
 static const struct swap_ops bdev_sync_swap_ops = {
 	.read_folio = swap_read_folio_bdev_sync,
-	.write_folio = swap_writepage_bdev_sync,
+	.write_folio = swap_write_folio_bdev_sync,
 };
 
 static const struct swap_ops bdev_async_swap_ops = {
 	.read_folio = swap_read_folio_bdev_async,
-	.write_folio = swap_writepage_bdev_async,
+	.write_folio = swap_write_folio_bdev_async,
 };
 
 void setup_swap_ops(struct swap_info_struct *sis)
-- 
2.39.3 (Apple Git-146)

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

[kernel test robot]

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

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

[kernel test robot]

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

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

[Barry Song]

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

Re: [PATCH v2 1/3] mm/swap: rename mm/page_io.c to mm/swap_io.c

[Chris Li]

Hi Barry,

Thanks for the patch.

Acked-by: Chris Li <chrisl@kernel.org>

Chris

On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
>
> From: Baoquan He <bhe@redhat.com>
>
> Codes in mm/page_io.c are only related to swap io, it has
> nothing to do with other page io.
>
> Rename it to avoid confusion.
>
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> Reviewed-by: Barry Song <baohua@kernel.org>
> Acked-by: Kairui Song <kasong@tencent.com>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Signed-off-by: Barry Song <baohua@kernel.org>
> ---
>  MAINTAINERS                 | 2 +-
>  mm/Makefile                 | 2 +-
>  mm/swap.h                   | 2 +-
>  mm/{page_io.c => swap_io.c} | 2 --
>  4 files changed, 3 insertions(+), 5 deletions(-)
>  rename mm/{page_io.c => swap_io.c} (99%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 16874c32e288..bb7c1031886d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16915,7 +16915,7 @@ F:      Documentation/mm/swap-table.rst
>  F:     include/linux/swap.h
>  F:     include/linux/swapfile.h
>  F:     include/linux/swapops.h
> -F:     mm/page_io.c
> +F:     mm/swap_io.c
>  F:     mm/swap.c
>  F:     mm/swap.h
>  F:     mm/swap_table.h
> diff --git a/mm/Makefile b/mm/Makefile
> index 8ad2ab08244e..a65ac900096a 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -75,7 +75,7 @@ ifdef CONFIG_MMU
>         obj-$(CONFIG_ADVISE_SYSCALLS)   += madvise.o
>  endif
>
> -obj-$(CONFIG_SWAP)     += page_io.o swap_state.o swapfile.o
> +obj-$(CONFIG_SWAP)     += swap_io.o swap_state.o swapfile.o
>  obj-$(CONFIG_ZSWAP)    += zswap.o
>  obj-$(CONFIG_HAS_DMA)  += dmapool.o
>  obj-$(CONFIG_HUGETLBFS)        += hugetlb.o hugetlb_sysfs.o hugetlb_sysctl.o
> diff --git a/mm/swap.h b/mm/swap.h
> index a77016f2423b..161185057993 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -214,7 +214,7 @@ extern void __swap_cluster_free_entries(struct swap_info_struct *si,
>                                         struct swap_cluster_info *ci,
>                                         unsigned int ci_off, unsigned int nr_pages);
>
> -/* linux/mm/page_io.c */
> +/* linux/mm/swap_io.c */
>  int sio_pool_init(void);
>  struct swap_iocb;
>  void swap_read_folio(struct folio *folio, struct swap_iocb **plug);
> diff --git a/mm/page_io.c b/mm/swap_io.c
> similarity index 99%
> rename from mm/page_io.c
> rename to mm/swap_io.c
> index 70cea9e24d2f..91b33d955e63 100644
> --- a/mm/page_io.c
> +++ b/mm/swap_io.c
> @@ -1,7 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - *  linux/mm/page_io.c
> - *
>   *  Copyright (C) 1991, 1992, 1993, 1994  Linus Torvalds
>   *
>   *  Swap reorganised 29.12.95,
> --
> 2.39.3 (Apple Git-146)
>

Re: [PATCH v2 3/3] mm/swap_io.c: rename swap_writepage_* to swap_write_folio_*

[Chris Li]

Acked-by: Chris Li <chrisl@kernel.org>

Chris

On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
>
> From: Baoquan He <bhe@redhat.com>
>
> All these swap_writepage_* functions are hanlding passed in folio, but
> not page. And this renaming make them consistent with the their
> counterpart swap_read_folio_* functions.
>
> Reviewed-by: Barry Song <baohua@kernel.org>
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Signed-off-by: Barry Song <baohua@kernel.org>
> ---
>  mm/swap_io.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/mm/swap_io.c b/mm/swap_io.c
> index ad0895af6ed8..063cd005ca0d 100644
> --- a/mm/swap_io.c
> +++ b/mm/swap_io.c
> @@ -375,7 +375,7 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
>         mempool_free(sio, sio_pool);
>  }
>
> -static void swap_writepage_fs(struct swap_info_struct *sis,
> +static void swap_write_folio_fs(struct swap_info_struct *sis,
>                               struct folio *folio,
>                               struct swap_iocb **swap_plug)
>  {
> @@ -412,7 +412,7 @@ static void swap_writepage_fs(struct swap_info_struct *sis,
>                 *swap_plug = sio;
>  }
>
> -static void swap_writepage_bdev_sync(struct swap_info_struct *sis,
> +static void swap_write_folio_bdev_sync(struct swap_info_struct *sis,
>                                      struct folio *folio,
>                                      struct swap_iocb **plug)
>  {
> @@ -433,7 +433,7 @@ static void swap_writepage_bdev_sync(struct swap_info_struct *sis,
>         __end_swap_bio_write(&bio);
>  }
>
> -static void swap_writepage_bdev_async(struct swap_info_struct *sis,
> +static void swap_write_folio_bdev_async(struct swap_info_struct *sis,
>                                       struct folio *folio,
>                                       struct swap_iocb **plug)
>  {
> @@ -595,17 +595,17 @@ static void swap_read_folio_bdev_async(struct swap_info_struct *sis,
>
>  static const struct swap_ops bdev_fs_swap_ops = {
>         .read_folio = swap_read_folio_fs,
> -       .write_folio = swap_writepage_fs,
> +       .write_folio = swap_write_folio_fs,
>  };
>
>  static const struct swap_ops bdev_sync_swap_ops = {
>         .read_folio = swap_read_folio_bdev_sync,
> -       .write_folio = swap_writepage_bdev_sync,
> +       .write_folio = swap_write_folio_bdev_sync,
>  };
>
>  static const struct swap_ops bdev_async_swap_ops = {
>         .read_folio = swap_read_folio_bdev_async,
> -       .write_folio = swap_writepage_bdev_async,
> +       .write_folio = swap_write_folio_bdev_async,
>  };
>
>  void setup_swap_ops(struct swap_info_struct *sis)
> --
> 2.39.3 (Apple Git-146)
>

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

[Baoquan He]

On 03/28/26 at 03:58pm, Barry Song wrote:
> From: Barry Song <baohua@kernel.org>
> 
> -v2:
>  * lots of cleanup for patch 2/3: renaming, moving data
>    structures, and using const properly
>  * collected tags from Kairui, Nhat and Barry

Thanks a lot for taking care of this, Barry. I took several days off for
family reason. Sorry for the inconvenience caused by my leave.

> 
> -v1:
>  https://lore.kernel.org/linux-mm/20260302104016.163542-1-bhe@redhat.com/
> 
> This can simplify the code logic and benefit any new type of swap device
> added later.
> 
> And also do renaming in this patchset:
> -------
>    file renaming:
>    ---
>    mm/page_io.c to mm/swap_io.c
> 
>    function renaming:
>    ---
>    swap_writepage_* to swap_write_folio_* in file mm/swap_io.c
> 
> Baoquan He (3):
>   mm/swap: rename mm/page_io.c to mm/swap_io.c
>   mm/swap: use swap_ops to register swap device's methods
>   mm/swap_io.c: rename swap_writepage_* to swap_write_folio_*
> 
>  MAINTAINERS                 |   2 +-
>  include/linux/swap.h        |   2 +
>  mm/Makefile                 |   2 +-
>  mm/swap.h                   |  12 ++++-
>  mm/{page_io.c => swap_io.c} | 101 ++++++++++++++++++++----------------
>  mm/swapfile.c               |   1 +
>  mm/zswap.c                  |   3 +-
>  7 files changed, 73 insertions(+), 50 deletions(-)
>  rename mm/{page_io.c => swap_io.c} (89%)
> 
> -- 
> 2.39.3 (Apple Git-146)
> 

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

[Chris Li]

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)
>

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

[Barry Song]

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

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

[Chris Li]

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

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

[Baoquan He]

On 03/30/26 at 08:30pm, Chris Li wrote:
......
> > @@ -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.

Thanks for your comments.

In the current patches, there's no chance any of these happens:
  sis->ops is NULL or
  op->read_folio == NULL or
  op->write_folio == NULL

Because it's a if-else logic in setup_swap_ops(), adding an error
checking looks a little weird. I think it's worth adding the error
checking in setup_swap_ops() later when we have new swap_ops added
and there's potential any of above three cases could happen.

What do you think?

Thanks
Baoquan

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

[Chris Li]

On Tue, Apr 7, 2026 at 11:11 PM Baoquan He <bhe@redhat.com> wrote:
>
> On 03/30/26 at 08:30pm, Chris Li wrote:
> ......
> > > @@ -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.
>
> Thanks for your comments.
>
> In the current patches, there's no chance any of these happens:
>   sis->ops is NULL or
>   op->read_folio == NULL or
>   op->write_folio == NULL

I am aware of that. Adding the check in setup_swap_ops() responds to
the WARN_ONCE check on op->write_folio() feedback.
In fact, we should never have a case where op->read_folio or
op->write_folio is NULL. However, I can see that other future swap ops
can be NULL. e.g. Notify swap entry free. It is hard to enforce that
some fields can be NULL and others can't. So having code safeguard it
is fine.

> Because it's a if-else logic in setup_swap_ops(), adding an error
> checking looks a little weird. I think it's worth adding the error
> checking in setup_swap_ops() later when we have new swap_ops added
> and there's potential any of above three cases could happen.

Adding the check in setup_swap_ops() is to remove the useless warning
check on the caller side of op->read_folio(). If you think the check
is not needed setup_swap_ops(), then it is not needed in
op->read_folio() either.
The WARN_ONCE() then kernel panic on reading op->read_folio does not
make sense. The check was added due to one of the feedback that what
if this call back is NULL.

I prefer to keep the `setup_swap_ops()` function responsible for the
NULL check, due to the discussion history surrounding it.
I'm also fine with adding the NULL check later. However, the NULL
check needs to be consistent across the patch series. The other NULL
check inside WARN_ONCE before the caller site op->read_folio()  and
op->write_folio() needs to be removed.

Does that make sense?

Chris

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

[Baoquan He]

On 04/08/26 at 02:26am, Chris Li wrote:
> On Tue, Apr 7, 2026 at 11:11 PM Baoquan He <bhe@redhat.com> wrote:
> >
> > On 03/30/26 at 08:30pm, Chris Li wrote:
> > ......
> > > > @@ -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.
> >
> > Thanks for your comments.
> >
> > In the current patches, there's no chance any of these happens:
> >   sis->ops is NULL or
> >   op->read_folio == NULL or
> >   op->write_folio == NULL
> 
> I am aware of that. Adding the check in setup_swap_ops() responds to
> the WARN_ONCE check on op->write_folio() feedback.
> In fact, we should never have a case where op->read_folio or
> op->write_folio is NULL. However, I can see that other future swap ops
> can be NULL. e.g. Notify swap entry free. It is hard to enforce that
> some fields can be NULL and others can't. So having code safeguard it
> is fine.
> 
> > Because it's a if-else logic in setup_swap_ops(), adding an error
> > checking looks a little weird. I think it's worth adding the error
> > checking in setup_swap_ops() later when we have new swap_ops added
> > and there's potential any of above three cases could happen.
> 
> Adding the check in setup_swap_ops() is to remove the useless warning
> check on the caller side of op->read_folio(). If you think the check
> is not needed setup_swap_ops(), then it is not needed in
> op->read_folio() either.
> The WARN_ONCE() then kernel panic on reading op->read_folio does not
> make sense. The check was added due to one of the feedback that what
> if this call back is NULL.
> 
> I prefer to keep the `setup_swap_ops()` function responsible for the
> NULL check, due to the discussion history surrounding it.
> I'm also fine with adding the NULL check later. However, the NULL
> check needs to be consistent across the patch series. The other NULL
> check inside WARN_ONCE before the caller site op->read_folio()  and
> op->write_folio() needs to be removed.
> 
> Does that make sense?

Both is fine to me. I will change to do the checking in
setup_swap_ops(). By the way, I tend to rename setup_swap_ops() as
init_swap_ops(), is it OK to you?

Thanks
Baoquan

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

[Chris Li]

On Wed, Apr 8, 2026 at 8:46 AM Baoquan He <bhe@redhat.com> wrote:
> Both is fine to me. I will change to do the checking in
> setup_swap_ops(). By the way, I tend to rename setup_swap_ops() as
> init_swap_ops(), is it OK to you?

Sure. Eventually we will need a function to detect the swap device and
decide which type of swap ops to use. This is just like in the file
system after reading the super block. The kernel need to probe which
file system driver to load.

Chris