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

[Chris Li]

On Thu, Apr 23, 2026 at 9:47 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
>
> On 04/23/26 at 02:48pm, Chris Li wrote:
> > The incremental change code logic looks fine.
> >
> > On Wed, Apr 22, 2026 at 7:37 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> > >
> > > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > > index af81fa212f1e..7644049a0919 100644
> > > --- a/mm/swapfile.c
> > > +++ b/mm/swapfile.c
> > > @@ -3518,10 +3518,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
> > >                 goto bad_swap_unlock_inode;
> > >         }
> > >
> > > -       if (init_swap_ops(si)) {
> > > -               error = -EINVAL;
> > > +       if (error = init_swap_ops(si))
> >
> > Some more nitpicks:
> >
> > I think some compiler option might warn about assigning a variable
> > inside an if condition. I suggest moving the assignment outside of the
> > if condition expression. As it is, it looks very close to "error ==
> > init_swap_ops(si))", Some compiler might warn, "Is that what you mean
> > instead?"
>
> I personally prefer the one doing assignment inside if condition
> expression because it is a ittle neater with one less line. While
> it doesn't matter much.

Yes, I understand it is a matter of personal preference. It will
generate the same code.

> Here comes the new version of the incremental update. Thanks.

LGTM.

Acked-by: Chris Li <chrisl@xxxxxxxxxx>

Chris

>
> diff --git a/mm/swap_io.c b/mm/swap_io.c
> index 77aa8373c087..e2710d5fb44e 100644
> --- a/mm/swap_io.c
> +++ b/mm/swap_io.c
> @@ -625,7 +625,7 @@ int init_swap_ops(struct swap_info_struct *sis)
>                 sis->ops = &bdev_async_swap_ops;
>
>         if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
> -               return -1;
> +               return -EINVAL;
>
>         return 0;
>  }
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index af81fa212f1e..82d2c9b35b11 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3518,10 +3518,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>                 goto bad_swap_unlock_inode;
>         }
>
> -       if (init_swap_ops(si)) {
> -               error = -EINVAL;
> +       error = init_swap_ops(si);
> +       if (error)
>                 goto bad_swap_unlock_inode;
> -       }
>
>         si->max = maxpages;
>         si->pages = maxpages - 1;
>