Re: [PATCH v8 1/2] mm/swap, PM: hibernate: fix swapoff race in uswsusp by pinning swap device

[YoungJun Park <youngjun.park@lge.com>]

On Tue, Mar 24, 2026 at 01:53:33PM +0800, Kairui Song wrote:
> On Tue, Mar 24, 2026 at 01:08:21AM +0800, Youngjun Park wrote:
> > Hibernation via uswsusp (/dev/snapshot ioctls) has a race window:
> > after selecting the resume swap area but before user space is frozen,
> > swapoff may run and invalidate the selected swap device.
> > 
> > Fix this by pinning the swap device with SWP_HIBERNATION while it is
> > in use.  The pin is exclusive, which is sufficient since
> > hibernate_acquire() already prevents concurrent hibernation sessions.
> > 
> > The kernel swsusp path (sysfs-based hibernate/resume) uses
> > find_hibernation_swap_type() which is not affected by the pin.  It
> > freezes user space before touching swap, so swapoff cannot race.
> > 
> > Introduce dedicated helpers:
> > - pin_hibernation_swap_type(): Look up and pin the swap device.
> >   Used by the uswsusp path.
> > - find_hibernation_swap_type(): Lookup without pinning.
> >   Used by the kernel swsusp path.
> > - unpin_hibernation_swap_type(): Clear the hibernation pin.
> 
> Looks good to me, thanks!
> 
> Reviewed-by: Kairui Song <kasong@tencent.com>

Thanks for the review, Kairui, and for all your feedback throughout
the revisions!

> Just one trivial nit picks below.

> > +/**
> > + * unpin_hibernation_swap_type - Unpin the swap device for hibernation
> > + * @type: Swap type previously returned by pin_hibernation_swap_type()
> > + *
> > + * Clear the hibernation pin on the given swap device, allowing
> > + * swapoff() to proceed normally.
> > + *
> > + * If @type does not refer to a valid swap device, this function
> > + * does nothing.
> > + */
> > +void unpin_hibernation_swap_type(int type)
> > +{
> > +	struct swap_info_struct *si;
> > +
> > +	spin_lock(&swap_lock);
> > +	si = swap_type_to_info(type);
> > +	if (!si) {
> > +		spin_unlock(&swap_lock);
> > +		return;
> > +	}
> > +	si->flags &= ~SWP_HIBERNATION;
> 
> Will the code will be simpler if you just:
> 
> if (si)
> 	si->flags &= ~SWP_HIBERNATION;
> 
> Just personal taste, free feel to ignore.

Noted on the style preference. I'll keep it in mind. :D

> And as you mentioned this is on top of swap table p3 so you based
> it on mm-new - but isn't p3 already in mm-unstable? Maybe we can
> have it there? Not sure how much conflict are there with PM.
> 
> The code and design looks OK.

Regarding the base branch. Andrew is already aware of the
potential conflicts in linux-next, and we've discussed possibly
parking this for the next cycle depending on Rafael's input.
So I think we can keep it as-is for now and see how things go!

Best regards,
Youngjun Park