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

[Baoquan He <baoquan.he@linux.dev>]

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@linux.dev> 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.

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

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;