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

[Baoquan He]

On 04/22/26 at 05:33pm, Chris Li wrote:
> On Tue, Apr 21, 2026 at 6:48 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> >
> > On 04/17/26 at 07:30pm, Chris Li wrote:
> > > On Thu, Apr 16, 2026 at 8:40 PM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> > ...snip...
> > > > +int init_swap_ops(struct swap_info_struct *sis)
> > > > +{
> > > > +       /*
> > > > +        * ->flags can be updated non-atomically, but that will
> > > > +        * never affect SWP_FS_OPS, so the data_race is safe.
> > > > +        */
> > > > +       if (data_race(sis->flags & SWP_FS_OPS))
> > > > +               sis->ops = &bdev_fs_swap_ops;
> > > > +       /*
> > > > +        * ->flags can be updated non-atomically, but that will
> > > > +        * never affect SWP_SYNCHRONOUS_IO, so the data_race is safe.
> > > > +        */
> > > > +       else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> > > > +               sis->ops = &bdev_sync_swap_ops;
> > > > +       else
> > > > +               sis->ops = &bdev_async_swap_ops;
> > > > +
> > > > +       if (!sis->ops || !sis->ops->read_folio || !sis->ops->write_folio)
> > > > +               return -1;
> > > Nitpick:
> > >
> > > For int type error return, you should use  -EINVAL or some thing with
> > > error code. If you don't care about error code, change the return type
> > > to bool instead.
> >
> > Thanks for careful reviewing, returning -EINVAL looks better, I will
> > change like that and repost.
> 
> You don't have to repost it as a new series. You can post one small
> incremental update for the -EINVAL change and ask Andrew to fold it
> in.
> 
> Hi Andrew,
> 
> I think this swap ops series can be merge to mm-unstable with the
> review. Keep in mind that Baoquan might have a very minor follow up
> patch relate to -EINAL error code.

The incremental update is as below. I have built and test it.

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..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))
 		goto bad_swap_unlock_inode;
-	}
 
 	si->max = maxpages;
 	si->pages = maxpages - 1;