| Message ID | 20260512053625.2950900-6-hch@lst.de |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [01/12] swap: remove the maxpages variable in sys_swapon | expand |
On 5/12/26 14:35, Christoph Hellwig wrote: > Reflow setup_swap_extents so that the flag checking is not conditional on > a swap_activate method. This is currently a no-op because the swapoff > code still checks the presence of a swap_deactivate method, but it > simplifies adding a new check, and also makes the SWP_ACTIVATED flag > more consistent. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks OK to me. Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
On Tue, May 12, 2026 at 07:35:21AM +0200, Christoph Hellwig wrote: > Reflow setup_swap_extents so that the flag checking is not conditional on > a swap_activate method. This is currently a no-op because the swapoff > code still checks the presence of a swap_deactivate method, but it > simplifies adding a new check, and also makes the SWP_ACTIVATED flag > more consistent. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > mm/swapfile.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 651c1b59ff9f..1b7fc03612f4 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -2783,25 +2783,24 @@ static int setup_swap_extents(struct swap_info_struct *sis, > { > struct address_space *mapping = swap_file->f_mapping; > struct inode *inode = mapping->host; > - int ret; > + int ret, error = 0; /me wonders why not reuse ret instead of declaring a new variable? --D > > if (S_ISBLK(inode->i_mode)) > return add_swap_extent(sis, sis->max, 0); > > - if (swap_file->f_op->swap_activate) { > + if (swap_file->f_op->swap_activate) > ret = swap_file->f_op->swap_activate(swap_file, sis); > - if (ret < 0) > - return ret; > - sis->flags |= SWP_ACTIVATED; > - if ((sis->flags & SWP_FS_OPS) && > - sio_pool_init() != 0) { > - destroy_swap_extents(sis, swap_file); > - return -ENOMEM; > - } > + else > + ret = generic_swap_activate(swap_file, sis); > + if (ret < 0) > return ret; > - } > > - return generic_swap_activate(swap_file, sis); > + sis->flags |= SWP_ACTIVATED; > + if (sis->flags & SWP_FS_OPS) > + error = sio_pool_init(); > + if (error) > + destroy_swap_extents(sis, swap_file); > + return error; > } > > static void _enable_swap_info(struct swap_info_struct *si) > -- > 2.53.0 > >
On Tue, May 12, 2026 at 09:43:08AM -0700, Darrick J. Wong wrote: > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 651c1b59ff9f..1b7fc03612f4 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -2783,25 +2783,24 @@ static int setup_swap_extents(struct swap_info_struct *sis, > > { > > struct address_space *mapping = swap_file->f_mapping; > > struct inode *inode = mapping->host; > > - int ret; > > + int ret, error = 0; > > /me wonders why not reuse ret instead of declaring a new variable? Because when I wrote this, the setup methods could still return a positive number of extents value that must not be clobbered. Since then I added patches before this that removed that, so we can use the same ret variable.
On Mon, May 11, 2026 at 10:37 PM Christoph Hellwig <hch@lst.de> wrote: > > Reflow setup_swap_extents so that the flag checking is not conditional on > a swap_activate method. This is currently a no-op because the swapoff > code still checks the presence of a swap_deactivate method, but it > simplifies adding a new check, and also makes the SWP_ACTIVATED flag > more consistent. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Chris Li <chrisl@kernel.org> Chris
diff --git a/mm/swapfile.c b/mm/swapfile.c index 651c1b59ff9f..1b7fc03612f4 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -2783,25 +2783,24 @@ static int setup_swap_extents(struct swap_info_struct *sis, { struct address_space *mapping = swap_file->f_mapping; struct inode *inode = mapping->host; - int ret; + int ret, error = 0; if (S_ISBLK(inode->i_mode)) return add_swap_extent(sis, sis->max, 0); - if (swap_file->f_op->swap_activate) { + if (swap_file->f_op->swap_activate) ret = swap_file->f_op->swap_activate(swap_file, sis); - if (ret < 0) - return ret; - sis->flags |= SWP_ACTIVATED; - if ((sis->flags & SWP_FS_OPS) && - sio_pool_init() != 0) { - destroy_swap_extents(sis, swap_file); - return -ENOMEM; - } + else + ret = generic_swap_activate(swap_file, sis); + if (ret < 0) return ret; - } - return generic_swap_activate(swap_file, sis); + sis->flags |= SWP_ACTIVATED; + if (sis->flags & SWP_FS_OPS) + error = sio_pool_init(); + if (error) + destroy_swap_extents(sis, swap_file); + return error; } static void _enable_swap_info(struct swap_info_struct *si)
Reflow setup_swap_extents so that the flag checking is not conditional on a swap_activate method. This is currently a no-op because the swapoff code still checks the presence of a swap_deactivate method, but it simplifies adding a new check, and also makes the SWP_ACTIVATED flag more consistent. Signed-off-by: Christoph Hellwig <hch@lst.de> --- mm/swapfile.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)