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

Baoquan He
2026-04-15T15:24:53Z

On 04/15/26 at 08:05am, Usama Arif wrote:
> On Wed, 15 Apr 2026 16:56:57 +0800 Baoquan He <baoquan.he@linux.dev> wrote:
>
> > This simplifies codes and makes logic clearer. And also makes later any
> > new swap device type being added easier to handle.
> >
> > Currently there are three types of swap devices: bdev_fs, bdev_sync
> > and bdev_async, and only operations read_folio and write_folio are
> > included. In the future, there could be more swap device types added
> > and more appropriate opeations adapted into swap_ops.
>
> s/opeations/operations/

Good catch, thanks.

>
> >
> > Suggested-by: Chris Li <chrisl@kernel.org>
> > Co-developed-by: Barry Song <baohua@kernel.org>
> > Signed-off-by: Barry Song <baohua@kernel.org>
> > Signed-off-by: Baoquan He <baoquan.he@linux.dev>
> > ---
> > include/linux/swap.h | 2 +
> > mm/swap.h | 10 ++++-
> > mm/swap_io.c | 102 +++++++++++++++++++++++++------------------
> > mm/swapfile.c | 5 +++
> > mm/zswap.c | 2 +-
> > 5 files changed, 76 insertions(+), 45 deletions(-)
> >
...snip...
> > diff --git a/mm/swap_io.c b/mm/swap_io.c
> > index 4bf210bd677e..602e9b871b30 100644
> > --- a/mm/swap_io.c
> > +++ b/mm/swap_io.c
...snip...
> > @@ -604,6 +588,44 @@ static void swap_read_folio_bdev_async(struct folio *folio,
> > submit_bio(bio);
> > }
> >
> > +static const struct swap_ops bdev_fs_swap_ops = {
> > + .read_folio = swap_read_folio_fs,
> > + .write_folio = swap_writepage_fs,
> > +};
> > +
> > +static const struct swap_ops bdev_sync_swap_ops = {
> > + .read_folio = swap_read_folio_bdev_sync,
> > + .write_folio = swap_writepage_bdev_sync,
> > +};
> > +
> > +static const struct swap_ops bdev_async_swap_ops = {
> > + .read_folio = swap_read_folio_bdev_async,
> > + .write_folio = swap_writepage_bdev_async,
> > +};
> > +
> > +int init_swap_ops(struct swap_info_struct *sis)
> > +{
> > + /*
> > + * ->flags can be updated non-atomically, but that will
> > + * never affect SWP_FS_OPS, so the data_race is safe.
> > + */
> > + if (data_race(sis->flags & SWP_FS_OPS))
> > + sis->ops = &bdev_fs_swap_ops;
> > + /*
> > + * ->flags can be updated non-atomically, but that will
> > + * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
> > + */
> > + else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> > + sis->ops = &bdev_sync_swap_ops;
> > + else
> > + sis->ops = &bdev_async_swap_ops;
> > +
> > + if (sis->ops || sis->ops->read_folio || sis->ops->write_folio)
> > + return -1;
>
> sis->ops is always non-NULL, and you will return -1 here and
> swapon will always fail with -EINVAL.
>
> You probably wanted?
>
> if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
> return -1;

You are right. Think one thing, write another. Will fix it in v4.
Thanks a lot for your careful reviewing.

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