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

[Barry Song <baohua@kernel.org>]

On Tue, Mar 31, 2026 at 4:31 PM Chris Li <chrisl@kernel.org> wrote:
>
> On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@gmail.com> wrote:
> >
> > From: Baoquan He <bhe@redhat.com>
> >
> > 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.
>
> Should add that there is no functional change expected from this patch.

Sounds good to me.

[...]

> > +static const struct swap_ops bdev_async_swap_ops = {
> > +       .read_folio = swap_read_folio_bdev_async,
> > +       .write_folio = swap_writepage_bdev_async,
> > +};
> > +
> > +void setup_swap_ops(struct swap_info_struct *sis)
>
> setup_swap_ops()` needs to return an indication of an error.
> The error is that sis->ops is NULL or op->read_folio == NULL or
> op->write_folio == NULL.
> If there is an error, we should fail the swapon.

Right now there is no possibility for any of the ops (read_folio or
write_folio) to be NULL, so I was using VM_WARN_ON as an assertion
to catch bugs in kernel code.

That said, I agree we can return an error earlier instead of letting
swapon fail later.

I think it may still make sense to keep a VM_WARN_ON in
setup_swap_ops() before returning -EINVAL or a similar error?

Thanks
Barry