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

[Kairui Song]

On Mon, May 11, 2026 at 3:43 PM Baoquan He <baoquan.he@xxxxxxxxx> 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.

Hi Baoquan,

Thanks for the patch. Sorry I was busy with travel and a few other
series and didn't got a chance to look at a few previous versions.

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

Overall, I really like this idea, we can have a cleaner interface
starting there.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 9174f1eeffb0..82d2c9b35b11 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3518,6 +3518,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>                 goto bad_swap_unlock_inode;
>         }
>
> +       error = init_swap_ops(si);
> +       if (error)
> +               goto bad_swap_unlock_inode;
> +

But this part seems wrong? init_swap_ops is called before
setup_swap_extents -> swap_activate sets SWP_FS_OPS, or
SWP_SYNCHRONOUS_IO is set below, so the branches in init_swap_ops
never take effect, am I right? You need to move this someplace after
these flags are set. Maybe right before swapon_mutex so the mutex is a
clean barrier that the swap device will be used and all things before
that will be seen by users, with some comments.