Re: [PATCH v5 3/3] PM: hibernate: fix spurious GFP mask WARNING in uswsusp path

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

On Thu, Mar 19, 2026 at 08:55:43PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 19, 2026 at 3:24 PM Youngjun Park <youngjun.park@lge.com> wrote:
> >
> > Commit 35e4a69b2003f ("PM: sleep: Allow pm_restrict_gfp_mask()
> > stacking") introduced refcount-based GFP mask management that warns
> > when pm_restore_gfp_mask() is called with saved_gfp_count == 0:
> >
> >   WARNING: kernel/power/main.c:44 at pm_restore_gfp_mask+0xd7/0xf0
> >   CPU: 0 UID: 0 PID: 373 Comm: s2disk
> >   Call Trace:
> >    snapshot_ioctl+0x964/0xbd0
> >    __x64_sys_ioctl+0x724/0x1320
> >   ...
> >
> > The uswsusp path calls pm_restore_gfp_mask() defensively in
> > SNAPSHOT_CREATE_IMAGE and SNAPSHOT_UNFREEZE where the GFP mask may
> > or may not be restricted depending on context (first call vs retry,
> > hibernate vs resume). Before the stacking patch this was a silent
> > no-op; now it triggers a WARNING.
> >
> > Introduce pm_restore_gfp_mask_safe() that skips the call when
> > saved_gfp_count is 0. This is preferred over tracking the restrict
> > state in snapshot_ioctl, as incorrect tracking risks leaving the
> > GFP mask permanently restricted.
> >
> > Fixes: 35e4a69b2003f ("PM: sleep: Allow pm_restrict_gfp_mask() stacking")
> > Signed-off-by: Youngjun Park <youngjun.park@lge.com>
> > ---
> >  include/linux/suspend.h | 1 +
> >  kernel/power/main.c     | 7 +++++++
> >  kernel/power/user.c     | 4 ++--
> >  3 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> > index b02876f1ae38..7777931d88a5 100644
> > --- a/include/linux/suspend.h
> > +++ b/include/linux/suspend.h
> > @@ -454,6 +454,7 @@ extern void pm_report_hw_sleep_time(u64 t);
> >  extern void pm_report_max_hw_sleep(u64 t);
> >  void pm_restrict_gfp_mask(void);
> >  void pm_restore_gfp_mask(void);
> > +void pm_restore_gfp_mask_safe(void);
> >
> >  #define pm_notifier(fn, pri) {                         \
> >         static struct notifier_block fn##_nb =                  \
> > diff --git a/kernel/power/main.c b/kernel/power/main.c
> > index 5f8c9e12eaec..e610a8c8b7ff 100644
> > --- a/kernel/power/main.c
> > +++ b/kernel/power/main.c
> > @@ -36,6 +36,13 @@
> >  static unsigned int saved_gfp_count;
> >  static gfp_t saved_gfp_mask;
> >
> > +void pm_restore_gfp_mask_safe(void)
> > +{
> > +       if (!saved_gfp_count)
> > +               return;
> > +       pm_restore_gfp_mask();
> > +}
> > +
> >  void pm_restore_gfp_mask(void)
> >  {
> >         WARN_ON(!mutex_is_locked(&system_transition_mutex));
> > diff --git a/kernel/power/user.c b/kernel/power/user.c
> > index 3e41544b99d5..41cff6a89a1c 100644
> > --- a/kernel/power/user.c
> > +++ b/kernel/power/user.c
> > @@ -306,7 +306,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> >         case SNAPSHOT_UNFREEZE:
> >                 if (!data->frozen || data->ready)
> >                         break;
> > -               pm_restore_gfp_mask();
> > +               pm_restore_gfp_mask_safe();
> >                 free_basic_memory_bitmaps();
> >                 data->free_bitmaps = false;
> >                 thaw_processes();
> > @@ -318,7 +318,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> >                         error = -EPERM;
> >                         break;
> >                 }
> > -               pm_restore_gfp_mask();
> > +               pm_restore_gfp_mask_safe();
> >                 error = hibernation_snapshot(data->platform_support);
> >                 if (!error) {
> >                         error = put_user(in_suspend, (int __user *)arg);
> > --
> 
> AFAICS, this patch doesn't depend on the rest of the series, so I can
> apply it separately unless there is a problem with that.
> 
> However, for the other 2 patches in the series, I'd need some tags
> (preferably Reviewed-by) from mm people.
> 
> Thanks!

Hi Rafael,

While double-checking the code based on Andrew’s AI-assisted review,
I noticed I missed one case.

If userspace issues SNAPSHOT_FREEZE and then closes the device,
snapshot_release() may call pm_restore_gfp_mask() without a matching
restriction, which reproduces the same WARN. So we should switch
snapshot_release() to pm_restore_gfp_mask_safe() as well.

Also, since the safe wrapper may return early when saved_gfp_count == 0,
the locking assertion would be skipped in that path. To preserve the
invariant, it is better to keep:

    WARN_ON(!mutex_is_locked(&system_transition_mutex));

in the wrapper too.
This modification is intentional, but after review I think this is better.

I will update the patch and resend.

Best regards,
Youngjun park