Re: [PATCH v7 3/3] mm/swap: add unplug callback to swap_ops to fix leaky abstraction

[Barry Song]

On Fri, May 15, 2026 at 9:58 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
>
> When swap_ops was introduced, the FS-swap batch submission remained
> as a standalone swap_write_unplug() that directly called
> mapping->a_ops->swap_rw(). This meant callers still had implicit
> knowledge of filesystem internals rather than going through the
> swap_ops abstraction.
>
> Fix this by adding an unplug callback to struct swap_ops.
> Each ops table provides its own implementation:
>  - bdev_fs_swap_ops uses the existing FS batch-submission logic
>  - bdev_sync/bdev_async_swap_ops leave it NULL since block-layer
>    plugging handles their I/O
>
> The swap_iocb now carries a pointer to its ops table so that
> swap_write_unplug() can dispatch through the callback without
> the caller needing to know the swap device type.
>
> Signed-off-by: Baoquan He <baoquan.he@xxxxxxxxx>
> ---
>  mm/page_io.c | 11 ++++++++++-
>  mm/swap.h    |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_io.c b/mm/page_io.c
> index 38b94c560c37..2c36d261ad98 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -329,6 +329,7 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio)
>
>  struct swap_iocb {
>         struct kiocb            iocb;
> +       const struct swap_ops   *ops;
>         struct bio_vec          bvec[SWAP_CLUSTER_MAX];
>         int                     pages;
>         int                     len;
> @@ -401,6 +402,7 @@ static void swap_fs_write_folio(struct swap_info_struct *sis,
>                 init_sync_kiocb(&sio->iocb, swap_file);
>                 sio->iocb.ki_complete = sio_write_complete;
>                 sio->iocb.ki_pos = pos;
> +               sio->ops = sis->ops;
>                 sio->pages = 0;
>                 sio->len = 0;
>         }
> @@ -454,7 +456,7 @@ static void swap_bdev_async_write_folio(struct swap_info_struct *sis,
>         submit_bio(bio);
>  }
>
> -void swap_write_unplug(struct swap_iocb *sio)
> +static void swap_fs_write_folio_unplug(struct swap_iocb *sio)
>  {
>         struct iov_iter from;
>         struct address_space *mapping = sio->iocb.ki_filp->f_mapping;
> @@ -466,6 +468,12 @@ void swap_write_unplug(struct swap_iocb *sio)
>                 sio_write_complete(&sio->iocb, ret);
>  }
>
> +void swap_write_unplug(struct swap_iocb *sio)
> +{
> +       if (sio->ops && sio->ops->unplug)
> +               sio->ops->unplug(sio);
> +}
> +

Hi Baoquan,

we have already "sis->ops" check in init_swap_ops, do we need !sis->ops
again?

+int init_swap_ops(struct swap_info_struct *sis)
+{
+ ...
+
+ if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
+ return -EINVAL;
+
+ return 0;
+}