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

[Chris Li]

On Tue, Mar 31, 2026 at 2:22 AM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> On Tue, Mar 31, 2026 at 4:31 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
> >
> > On Sat, Mar 28, 2026 at 12:58 AM Barry Song <21cnbao@xxxxxxxxx> wrote:
> > >
> > > From: Baoquan He <bhe@xxxxxxxxxx>
> > >
> > > 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.

Ack

>
> > > +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.

Ack. Someone might register a new swap_ops later that introduces a
NULL swap_write_folio. The check is for catching that.

>
> 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?

Sure. We can just print  pr_err() rather than VM_WARN_ON.  VM_WARN_ON
is nop when VM debug is disabled. It is a kernel swap code bug on the
swapon path, so the back trace portion of the warning is not very
valuable. A normal error print will suffice. Because the swapon
failed, the user will notice this error.

Chris