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

["Rafael J. Wysocki" <rafael@kernel.org>]

Again, this patch does not appear to depend on the rest of the series,
in which case there's no need to send the other patches along with it.

Please send the next version separately unless there is a dependency
on the other two patches, but in that case that dependency should be
explained in the changelog.

On Fri, Mar 20, 2026 at 6:03 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, SNAPSHOT_UNFREEZE, and snapshot_release(),
> where the GFP mask may or may not be restricted depending on the
> execution path.
>
> Before the stacking change this was a silent no-op; it now triggers
> a WARNING when saved_gfp_count is 0.
>
> Introduce pm_restore_gfp_mask_safe(), which skips the call if

It would be better to call this pm_restore_gfp_mask_nowarn() IMV.

> saved_gfp_count is 0. This avoids the warning without requiring
> state tracking in snapshot_ioctl, which could otherwise leave the
> GFP mask permanently restricted if mismanaged.
>
> 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     | 18 ++++++++++++++++++
>  kernel/power/user.c     |  6 +++---
>  3 files changed, 22 insertions(+), 3 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..90e9bd56a433 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -36,6 +36,24 @@
>  static unsigned int saved_gfp_count;
>  static gfp_t saved_gfp_mask;
>
> +/**
> + * pm_restore_gfp_mask_safe - Conditionally restore the GFP mask
> + *
> + * Call pm_restore_gfp_mask() only if a GFP restriction is active.
> + *
> + * After GFP mask stacking was introduced, calling
> + * pm_restore_gfp_mask() without a matching restriction triggers a
> + * warning. Some hibernation paths invoke restore defensively, so this
> + * helper avoids spurious warnings when no restriction is in place.
> + */
> +void pm_restore_gfp_mask_safe(void)
> +{
> +       WARN_ON(!mutex_is_locked(&system_transition_mutex));

Could this be changed to lockdep_assert_held()?

> +       if (!saved_gfp_count)
> +               return;

Please add an empty line here.

> +       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 aab9aece1009..88de4b76a9dc 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -119,7 +119,7 @@ static int snapshot_release(struct inode *inode, struct file *filp)
>         free_all_swap_pages(data->swap);
>         unpin_hibernation_swap_type(data->swap);
>         if (data->frozen) {
> -               pm_restore_gfp_mask();
> +               pm_restore_gfp_mask_safe();
>                 free_basic_memory_bitmaps();
>                 thaw_processes();
>         } else if (data->free_bitmaps) {
> @@ -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);
> --