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

[Baoquan He <bhe@redhat.com>]

On 03/02/26 at 11:35am, Chris Li wrote:
> On Mon, Mar 2, 2026 at 6:10 AM YoungJun Park <youngjun.park@lge.com> wrote:
> >
> > On Mon, Mar 02, 2026 at 06:40:15PM +0800, Baoquan He 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.
> > >
> > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > ---
> > >  include/linux/swap.h |  13 ++++++
> > >  mm/swap.h            |   1 -
> > >  mm/swap_io.c         | 102 +++++++++++++++++++++++++------------------
> > >  mm/swapfile.c        |   2 +
> > >  mm/zswap.c           |   3 +-
> > >  5 files changed, 76 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > > index 0effe3cc50f5..448e5e66ec5c 100644
> > > --- a/include/linux/swap.h
> > > +++ b/include/linux/swap.h
> > > @@ -19,6 +19,7 @@
> > >  struct notifier_block;
> > >
> > >  struct bio;
> > > +struct swap_iocb;
> > >
> > >  struct pagevec;
> > >
> > > @@ -222,6 +223,17 @@ enum {
> > >  #define SWAP_CLUSTER_MAX_SKIPPED (SWAP_CLUSTER_MAX << 10)
> > >  #define COMPACT_CLUSTER_MAX SWAP_CLUSTER_MAX
> > >
> > > +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);
> > > +};
> >
> > I think swap_iocb is only required for fs-swap
> > (swap_folio_read_fs/swap_folio_write_fs).
> >
> > If the goal is to support fs-swap through swap_ops, it might be worth
> 
> Consider this series as a starting point for discussion. Nothing is
> set to stone.
> 
> > considering a more complete integration, including activate/deactivate
> > and swap_rw from aops, rather than only adding read/write hooks.
> 
> Can be add as follow up patches to introduce active/deactive and
> swap_rw from aops. In fact I want to see it as incremental patches
> rather than add every possible swap_ops in one go. It is likely easier
> to review as well.

Ack. And thanks for bring this up to Chris, I should add Chris's
Suggested-by.

> >
> > So.. we could keep SWP_FS_OPS as-is for now and just split
> > sync/async paths, and revisit a cleaner fs-swap integration later.
> > (I mean removing fs ops, and call swap_read/write_folio_fs on sync/async ops.)
> 
> You can propose incremental patches to add your additional swap ops
> and into the series as the later iteration.
> The sync/async split is just a MVP to introduce the first step of the swap_ops.

Agree.