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

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

Changlog:
===
-v6:
 * Fix a code bug Kairui found out when reviewing patch in mm/swapfile.c
   in patch 2/3. Has fixed it by moving it to appropriate place and add
   comment to explain.

-v5:
 * Change the return value of init_swap_ops() as -EINVAL as per Chris's
   suggestion and adjust its invocation in swapon() accordingly.
 * Add Chris and Usama's Ack tags.

-v4:
 * Fix a typo opeations -> operations
 * Fix a code bug inside init_swap_ops(). I was taking a change at the
   time, thought the change is trivial, so I only compiled but didn't
   run kernel to test in v3. Now fix it and test passed.
   Thanaks to Usama for catching the above two issues.

-v3:
 * Rename setup_swap_ops() to init_swap_ops() which reflect the function
   behaviour a little better
 * Check if sis->ops, sis->ops->read_folio and sis->ops->write_folio is
   NULL in init_swap_ops(), but not spread them where they are called.
   And once the checking failed, fail swapon immediately. This is
   suggested by Chris.
 * Call init_swap_ops() before setup_swap_extents() invocation. This
   doesn't harm anything and can benefit later adding sis->ops->swap_activate
   method.

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

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} | 104 ++++++++++++++++++++----------------
 mm/swapfile.c               |   9 ++++
 mm/zswap.c                  |   2 +-
 7 files changed, 83 insertions(+), 50 deletions(-)
 rename mm/{page_io.c => swap_io.c} (90%)

-- 
2.52.0

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 <baoquan.he@linux.dev>
---
 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 98d0a7a1c689..b7746b359471 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS

@@ -17033,7 +17033,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 7ed76592e20d..7c1a3a3a53f8 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.52.0

On Tue, May 12, 2026 at 3:42 AM Baoquan He [off-list ref] wrote:

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 <baoquan.he@linux.dev>

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

Chris

On Tue, May 12, 2026 at 06:41:59PM +0800, Baoquan He wrote:

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

That is true.  At the same time it also (mostly) isn't about all swap
I/O, but specifically about the block based I/O backend for swap, and
secondarily a little bit about helpers for the swsusp I/O to the swap
file, with a little bit generic swap I/O wrappers thrown in.

So the new new isn't grest either.  And I'd rather wait with this until
we can nicely split stuff out - the rest of this series is a good step
toward that, and my swap_activate series is another.  After that we
should be able to stop creating swap_extent structures for
SWP_FS_OPS-based I/O, and contain struct swap_extent with the bio code
and actually create a coherent abstraction.  This will require moving
a bit more code around, though.  My preference for the resulting name
would be swap-block.c or swap-bio.c, but that's up for future
discussion.

Can we just skip this for now?

On 05/12/26 at 10:21pm, Christoph Hellwig wrote:

On Tue, May 12, 2026 at 06:41:59PM +0800, Baoquan He wrote:

quoted

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

That is true.  At the same time it also (mostly) isn't about all swap
I/O, but specifically about the block based I/O backend for swap, and
secondarily a little bit about helpers for the swsusp I/O to the swap
file, with a little bit generic swap I/O wrappers thrown in.

So the new new isn't grest either.  And I'd rather wait with this until

"So the new isn't great either." ?

Take it easy, I can feel this abstraction falls into your area?

we can nicely split stuff out - the rest of this series is a good step
toward that, and my swap_activate series is another.  After that we
should be able to stop creating swap_extent structures for
SWP_FS_OPS-based I/O, and contain struct swap_extent with the bio code
and actually create a coherent abstraction.  This will require moving
a bit more code around, though.  My preference for the resulting name
would be swap-block.c or swap-bio.c, but that's up for future
discussion.

Can we just skip this for now?

Yes, I agree with you, and the swap_extent creating thing sounds great,
look forward to seeing it. I will drop this patch 1/3 in v7.

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>
Acked-by: Chris Li <chrisl@kernel.org>
Co-developed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Barry Song <baohua@kernel.org>
Signed-off-by: Baoquan He <baoquan.he@linux.dev>
---
 include/linux/swap.h |   2 +
 mm/swap.h            |  10 ++++-
 mm/swap_io.c         | 102 +++++++++++++++++++++++++------------------
 mm/swapfile.c        |   9 ++++
 mm/zswap.c           |   2 +-
 5 files changed, 80 insertions(+), 45 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index aa89e1d30a77..85f15ec326c1 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..29bdc679fa98 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);
+};
+int init_swap_ops(struct swap_info_struct *sis);
 void swap_read_folio(struct folio *folio, struct swap_iocb **plug);
 void __swap_read_unplug(struct swap_iocb *plug);
 static inline void swap_read_unplug(struct swap_iocb *plug)

@@ -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 7c1a3a3a53f8..3f9aee5b6a4d 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,7 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
 	}
 	rcu_read_unlock();
 
-	__swap_writepage(folio, swap_plug);
+	sis->ops->write_folio(sis, folio, swap_plug);
 	return 0;
 out_unlock:
 	folio_unlock(folio);

@@ -373,10 +374,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 +411,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 +432,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 +450,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 +518,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 +553,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 +576,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 +592,44 @@ static void swap_read_folio_bdev_async(struct folio *folio,
 	submit_bio(bio);
 }
 
+static const struct swap_ops bdev_fs_swap_ops = {
+	.read_folio = swap_read_folio_fs,
+	.write_folio = swap_writepage_fs,
+};
+
+static const struct swap_ops bdev_sync_swap_ops = {
+	.read_folio = swap_read_folio_bdev_sync,
+	.write_folio = swap_writepage_bdev_sync,
+};
+
+static const struct swap_ops bdev_async_swap_ops = {
+	.read_folio = swap_read_folio_bdev_async,
+	.write_folio = swap_writepage_bdev_async,
+};
+
+int init_swap_ops(struct swap_info_struct *sis)
+{
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_FS_OPS, so the data_race is safe.
+	 */
+	if (data_race(sis->flags & SWP_FS_OPS))
+		sis->ops = &bdev_fs_swap_ops;
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
+	 */
+	else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
+		sis->ops = &bdev_sync_swap_ops;
+	else
+		sis->ops = &bdev_async_swap_ops;
+
+	if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
+		return -EINVAL;
+
+	return 0;
+}
+
 void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
 {
 	struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);

@@ -642,13 +664,7 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
 	/* We have to read from slower devices. Increase zswap protection. */
 	zswap_folio_swapin(folio);
 
-	if (data_race(sis->flags & SWP_FS_OPS)) {
-		swap_read_folio_fs(folio, plug);
-	} else if (synchronous) {
-		swap_read_folio_bdev_sync(folio, sis);
-	} else {
-		swap_read_folio_bdev_async(folio, sis);
-	}
+	sis->ops->read_folio(sis, folio, plug);
 
 finish:
 	if (workingset) {

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4840fd40f36f..8c42632e6765 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c

@@ -3780,6 +3780,15 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 		goto free_swap_zswap;
 	}
 
+	/*
+	 * init_swap_ops() sets si->ops based on flags. It does not need
+	 * swapon_mutex, and must complete before enable_swap_info()
+	 * exposes the device.
+	 */
+	error = init_swap_ops(si);
+	if (error)
+		goto bad_swap_unlock_inode;
+
 	mutex_lock(&swapon_mutex);
 	prio = DEF_SWAP_PRIO;
 	if (swap_flags & SWAP_FLAG_PREFER)

diff --git a/mm/zswap.c b/mm/zswap.c
index 4b5149173b0e..192401f46de4 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 */
-	__swap_writepage(folio, NULL);
+	si->ops->write_folio(si, folio, NULL);
 
 out:
 	if (ret && ret != -EEXIST) {

-- 
2.52.0

On Tue, May 12, 2026 at 6:50 PM Baoquan He [off-list ref] wrote:

This simplifies codes and makes logic clearer. And also makes later any
new swap device type being added easier to handle.

Currently there are three types of swap devices: bdev_fs, bdev_sync
and bdev_async, and only operations read_folio and write_folio are
included. In the future, there could be more swap device types added
and more appropriate opeations adapted into swap_ops.

opeations -> operations

Suggested-by: Chris Li <chrisl@kernel.org>
Acked-by: Chris Li <chrisl@kernel.org>
Co-developed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Barry Song <baohua@kernel.org>
Signed-off-by: Baoquan He <baoquan.he@linux.dev>

A few nitpicks below:

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

This sanify check is dropped and not added back in anywhere. This is
fine, but it might be better to have a similar VM_WARN_ON_FOLIO in
swap_writeout?

I checked the comment above previously and it looked good. But the
error label seems not that correct after double check. inode->i_flags
will keep the S_SWAPFILE flag. Maybe something like add a
inode->i_flags &= ~S_SWAPFILE here and goto free_swap_zswap. Sorry I
didn't check this part carefully last time.

But fortunately, init_swap_ops will never fail at this moment so the
issue never triggers.

Rest looks good to me!

On 05/13/26 at 01:53am, Kairui Song wrote:

On Tue, May 12, 2026 at 6:50 PM Baoquan He [off-list ref] wrote:

quoted

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.

opeations -> operations

Will fix, thanks.

quoted

Suggested-by: Chris Li <chrisl@kernel.org>
Acked-by: Chris Li <chrisl@kernel.org>
Co-developed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Barry Song <baohua@kernel.org>
Signed-off-by: Baoquan He <baoquan.he@linux.dev>

A few nitpicks below:

quoted

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

This sanify check is dropped and not added back in anywhere. This is
fine, but it might be better to have a similar VM_WARN_ON_FOLIO in
swap_writeout?

Good catch. In the old code, it's called by both swap_writeout() and
zswap_writeback_entry(). In zswap_writeback_entry(), the folio is
allocated in-place, we don't need to worry about it now. I am fine to
add it back in swap_writeout() because adding it inside all three
swap_write_xxx() is a little too much.

Cscope tag: __swap_writepage
   #   line  filename / context / line
   1    288  /home/bhe/code/linux/mm/page_io.c <<swap_writeout>>
             __swap_writepage(folio, swap_plug);
   2   1053  /home/bhe/code/linux/mm/zswap.c <<zswap_writeback_entry>>
             __swap_writepage(folio, NULL);

quoted

 void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
 {
        struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);

@@ -642,13 +664,7 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
        /* We have to read from slower devices. Increase zswap protection. */
        zswap_folio_swapin(folio);

-       if (data_race(sis->flags & SWP_FS_OPS)) {
-               swap_read_folio_fs(folio, plug);
-       } else if (synchronous) {
-               swap_read_folio_bdev_sync(folio, sis);
-       } else {
-               swap_read_folio_bdev_async(folio, sis);
-       }
+       sis->ops->read_folio(sis, folio, plug);

 finish:
        if (workingset) {

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4840fd40f36f..8c42632e6765 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c

@@ -3780,6 +3780,15 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
                goto free_swap_zswap;
        }

+       /*
+        * init_swap_ops() sets si->ops based on flags. It does not need
+        * swapon_mutex, and must complete before enable_swap_info()
+        * exposes the device.
+        */
+       error = init_swap_ops(si);
+       if (error)
+               goto bad_swap_unlock_inode;

I checked the comment above previously and it looked good. But the
error label seems not that correct after double check. inode->i_flags
will keep the S_SWAPFILE flag. Maybe something like add a
inode->i_flags &= ~S_SWAPFILE here and goto free_swap_zswap. Sorry I
didn't check this part carefully last time.

Right. How about moving it zswap_swapon() because it only relies on
si->flags setting currently?

But fortunately, init_swap_ops will never fail at this moment so the
issue never triggers.

quoted

+
        mutex_lock(&swapon_mutex);
        prio = DEF_SWAP_PRIO;
        if (swap_flags & SWAP_FLAG_PREFER)

diff --git a/mm/zswap.c b/mm/zswap.c
index 4b5149173b0e..192401f46de4 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 */
-       __swap_writepage(folio, NULL);
+       si->ops->write_folio(si, folio, NULL);

 out:
        if (ret && ret != -EEXIST) {
--

2.52.0

Rest looks good to me!

On Thu, May 14, 2026 at 10:06 AM Baoquan He [off-list ref] wrote:

On 05/13/26 at 01:53am, Kairui Song wrote:

quoted

On Tue, May 12, 2026 at 6:50 PM Baoquan He [off-list ref] wrote:

quoted

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.

opeations -> operations

Will fix, thanks.

quoted

quoted

Suggested-by: Chris Li <chrisl@kernel.org>
Acked-by: Chris Li <chrisl@kernel.org>
Co-developed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Barry Song <baohua@kernel.org>
Signed-off-by: Baoquan He <baoquan.he@linux.dev>

A few nitpicks below:

quoted

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

This sanify check is dropped and not added back in anywhere. This is
fine, but it might be better to have a similar VM_WARN_ON_FOLIO in
swap_writeout?

Good catch. In the old code, it's called by both swap_writeout() and
zswap_writeback_entry(). In zswap_writeback_entry(), the folio is
allocated in-place, we don't need to worry about it now. I am fine to
add it back in swap_writeout() because adding it inside all three
swap_write_xxx() is a little too much.

Cscope tag: __swap_writepage
   #   line  filename / context / line
   1    288  /home/bhe/code/linux/mm/page_io.c <<swap_writeout>>
             __swap_writepage(folio, swap_plug);
   2   1053  /home/bhe/code/linux/mm/zswap.c <<zswap_writeback_entry>>
             __swap_writepage(folio, NULL);

quoted

quoted

 void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
 {
        struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);

@@ -642,13 +664,7 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
        /* We have to read from slower devices. Increase zswap protection. */
        zswap_folio_swapin(folio);

-       if (data_race(sis->flags & SWP_FS_OPS)) {
-               swap_read_folio_fs(folio, plug);
-       } else if (synchronous) {
-               swap_read_folio_bdev_sync(folio, sis);
-       } else {
-               swap_read_folio_bdev_async(folio, sis);
-       }
+       sis->ops->read_folio(sis, folio, plug);

 finish:
        if (workingset) {

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4840fd40f36f..8c42632e6765 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c

@@ -3780,6 +3780,15 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
                goto free_swap_zswap;
        }

+       /*
+        * init_swap_ops() sets si->ops based on flags. It does not need
+        * swapon_mutex, and must complete before enable_swap_info()
+        * exposes the device.
+        */
+       error = init_swap_ops(si);
+       if (error)
+               goto bad_swap_unlock_inode;

I checked the comment above previously and it looked good. But the
error label seems not that correct after double check. inode->i_flags
will keep the S_SWAPFILE flag. Maybe something like add a
inode->i_flags &= ~S_SWAPFILE here and goto free_swap_zswap. Sorry I
didn't check this part carefully last time.

Right. How about moving it zswap_swapon() because it only relies on
si->flags setting currently?

You mean before zswap_swapon()? Then that sounds good to me.

On 05/14/26 at 02:41pm, Kairui Song wrote:

On Thu, May 14, 2026 at 10:06 AM Baoquan He [off-list ref] wrote:

quoted

On 05/13/26 at 01:53am, Kairui Song wrote:

quoted

On Tue, May 12, 2026 at 6:50 PM Baoquan He [off-list ref] wrote:

......snip...

quoted

quoted

quoted

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4840fd40f36f..8c42632e6765 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c

@@ -3780,6 +3780,15 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
                goto free_swap_zswap;
        }

+       /*
+        * init_swap_ops() sets si->ops based on flags. It does not need
+        * swapon_mutex, and must complete before enable_swap_info()
+        * exposes the device.
+        */
+       error = init_swap_ops(si);
+       if (error)
+               goto bad_swap_unlock_inode;

I checked the comment above previously and it looked good. But the
error label seems not that correct after double check. inode->i_flags
will keep the S_SWAPFILE flag. Maybe something like add a
inode->i_flags &= ~S_SWAPFILE here and goto free_swap_zswap. Sorry I
didn't check this part carefully last time.

Right. How about moving it zswap_swapon() because it only relies on
si->flags setting currently?

You mean before zswap_swapon()? Then that sounds good to me.

Yeah, just before zswap_swapon(). I missed a word.

 };
 
+struct swap_ops;
 /*

Please keep empty spaces before comments.  And keeping forward
declarations at the beginning of the file makes them much easier to
organize (although the current swap.h is a complete mess not only in this
regard).

+int init_swap_ops(struct swap_info_struct *sis)
+{
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_FS_OPS, so the data_race is safe.
+	 */
+	if (data_race(sis->flags & SWP_FS_OPS))
+		sis->ops = &bdev_fs_swap_ops;
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
+	 */
+	else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
+		sis->ops = &bdev_sync_swap_ops;
+	else
+		sis->ops = &bdev_async_swap_ops;
+
+	if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
+		return -EINVAL;

In the long run setting these from the helpers for ->swap_activate would
make more sense.  Also as ls long as the ops are a small statically
defined set these checks here are a bit odd.

I guess we just need to land this ASAP, then my swap_activate series and
then do a big round of cleanups for a coherent swap interface.

-	} else if (synchronous) {
-		swap_read_folio_bdev_sync(folio, sis);
-	} else {
-		swap_read_folio_bdev_async(folio, sis);
-	}
+	sis->ops->read_folio(sis, folio, plug);

This is a really annoying double-indirection for the SWP_FS_OPS
case.  I think we should have one level of indirection here.  One
way to do this would be to remove the swap_rw operation and
implement the swap_ops directly in nfs and cifs.

On 05/12/26 at 10:32pm, Christoph Hellwig wrote:

quoted

 };
 
+struct swap_ops;
 /*

Please keep empty spaces before comments.  And keeping forward

 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I may not follow, I noticed that all the comments in the code use
tabs for indentation rather than spaces.

declarations at the beginning of the file makes them much easier to
organize (although the current swap.h is a complete mess not only in this
regard).

Sure, will move it at the beginning of linux/swap.h. Thanks.

quoted

+int init_swap_ops(struct swap_info_struct *sis)
+{
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_FS_OPS, so the data_race is safe.
+	 */
+	if (data_race(sis->flags & SWP_FS_OPS))
+		sis->ops = &bdev_fs_swap_ops;
+	/*
+	 * ->flags can be updated non-atomically, but that will
+	 * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
+	 */
+	else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
+		sis->ops = &bdev_sync_swap_ops;
+	else
+		sis->ops = &bdev_async_swap_ops;
+
+	if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
+		return -EINVAL;

In the long run setting these from the helpers for ->swap_activate would
make more sense.  Also as ls long as the ops are a small statically
defined set these checks here are a bit odd.

In fact, except of your ->swap_activate, I have my ->swap_activate too
queued in my local branch. While from your comments, seems we don't have
conflict because we have different concerns.

Please see Youngjun's comment and my reply in this thread:
https://lore.kernel.org/linux-mm/aaWiBaeSCILBCzqd@yjaykim-PowerEdge-T330/

I guess we just need to land this ASAP, then my swap_activate series and
then do a big round of cleanups for a coherent swap interface.

This patchset is part I. I posted v1 to collect suggestions, later I
took a leave, then people think the v1 can be merged firstly and helped
to post v2. I later came back and continue to push this part. If you
just found this patch series and come up with those cleanups, could you
wait a moment until we merge this part and part II is posted? I think
the split handling of block and fs in swap you mentioned is super cool,
and is beyond our plan.

quoted

-	} else if (synchronous) {
-		swap_read_folio_bdev_sync(folio, sis);
-	} else {
-		swap_read_folio_bdev_async(folio, sis);
-	}
+	sis->ops->read_folio(sis, folio, plug);

This is a really annoying double-indirection for the SWP_FS_OPS
case.  I think we should have one level of indirection here.  One
way to do this would be to remove the swap_rw operation and
implement the swap_ops directly in nfs and cifs.

On Wed, May 13, 2026 at 11:33:50PM +0800, Baoquan He wrote:

This patchset is part I. I posted v1 to collect suggestions, later I
took a leave, then people think the v1 can be merged firstly and helped
to post v2. I later came back and continue to push this part. If you
just found this patch series and come up with those cleanups, could you
wait a moment until we merge this part and part II is posted? I think
the split handling of block and fs in swap you mentioned is super cool,
and is beyond our plan.

I've been looking at this a bit more, and I think it is going very
much in the wrong direction.  I've started drafting a version that
moves block to the same model as fs to build up multi-folio bios,
which actually allows for a clean abstraction.

On Thu, May 14, 2026 at 11:30 PM Christoph Hellwig [off-list ref] wrote:

On Wed, May 13, 2026 at 11:33:50PM +0800, Baoquan He wrote:

quoted

This patchset is part I. I posted v1 to collect suggestions, later I
took a leave, then people think the v1 can be merged firstly and helped
to post v2. I later came back and continue to push this part. If you
just found this patch series and come up with those cleanups, could you
wait a moment until we merge this part and part II is posted? I think
the split handling of block and fs in swap you mentioned is super cool,
and is beyond our plan.

I've been looking at this a bit more, and I think it is going very
much in the wrong direction.  I've started drafting a version that
moves block to the same model as fs to build up multi-folio bios,
which actually allows for a clean abstraction.

That is for the swapin_readahead() where multiple folios can be read
in one batch, right? I assume It should not impact the swap out path.
Curious to see what you have in mind. The swap_vma_readahead()  can
hit more than one swap device. The current swap_ops is per swap device
so your multi-folio bios will likely stay in the core swap layer. I
will take a look at your new implementation; hopefully, the swap_ops
can adapt to that as well.

Chris

On Fri, May 15, 2026 at 2:31 PM Chris Li [off-list ref] wrote:

quoted

I've been looking at this a bit more, and I think it is going very
much in the wrong direction.  I've started drafting a version that
moves block to the same model as fs to build up multi-folio bios,
which actually allows for a clean abstraction.

That is for the swapin_readahead() where multiple folios can be read
in one batch, right? I assume It should not impact the swap out path.
Curious to see what you have in mind. The swap_vma_readahead()  can
hit more than one swap device. The current swap_ops is per swap device
so your multi-folio bios will likely stay in the core swap layer. I
will take a look at your new implementation; hopefully, the swap_ops
can adapt to that as well.

Never mind that; I just saw your swap batching series. I will reply there.

Chris

Looking a bit more I also think this abastraction is still leaky because
the unplug calls for fs based swap aren't properly abstracted out.

(note that it would make sense to use them for block based swap as well,
as building a single bio will be a lot more efficient than relying on
block layer plugging, but that's a separate issue)

On 05/12/26 at 10:45pm, Christoph Hellwig wrote:

Looking a bit more I also think this abastraction is still leaky because
the unplug calls for fs based swap aren't properly abstracted out.

Right, and this can be easily done by adding a unplug callback in
swap_ops.

(note that it would make sense to use them for block based swap as well,
as building a single bio will be a lot more efficient than relying on
block layer plugging, but that's a separate issue)

OK, look forward to seeing it from your post.

All these swap_writepage_* functions handle a passed-in folio,
not a page. This renaming makes them consistent with 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 <baoquan.he@linux.dev>
---
 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 3f9aee5b6a4d..ff38adfde0fa 100644
--- a/mm/swap_io.c
+++ b/mm/swap_io.c

@@ -374,7 +374,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)
 {

@@ -411,7 +411,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)
 {

@@ -432,7 +432,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)
 {

@@ -594,17 +594,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,
 };
 
 int init_swap_ops(struct swap_info_struct *sis)

-- 
2.52.0

On Tue, May 12, 2026 at 6:42 PM Baoquan He [off-list ref] wrote:

All these swap_writepage_* functions handle a passed-in folio,
not a page. This renaming makes them consistent with 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 <baoquan.he@linux.dev>
---
 mm/swap_io.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Acked-by: Kairui Song <kasong@tencent.com>

On Tue, May 12, 2026 at 3:42 AM Baoquan He [off-list ref] wrote:

All these swap_writepage_* functions handle a passed-in folio,
not a page. This renaming makes them consistent with 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 <baoquan.he@linux.dev>

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

Chris

 
 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,
 };

The convention for method naming is prefix##method_name, i.e.
swap_fs_read_folio, swap_fs_write_folio, swap_bdev_sync_read_folios,
etc.

On 05/12/26 at 10:36pm, Christoph Hellwig wrote:

quoted

 
 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,
 };

The convention for method naming is prefix##method_name, i.e.
swap_fs_read_folio, swap_fs_write_folio, swap_bdev_sync_read_folios,
etc.

To me _fs is a modifier to qualify the method type. If need change, we
may need change all swap_readxxx_fs/bdev_sync/bdev_async and swap_writexxx
likewise.