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

YoungJun Park
2026-03-24T12:48:12Z

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