Re: [PATCH v2 1/3] PM: hibernate: Fix hybrid-sleep

["Mario Limonciello (AMD) (kernel.org)" <superm1@kernel.org>]

On 9/25/2025 12:55 PM, Rafael J. Wysocki wrote:
> On Thu, Sep 25, 2025 at 7:51 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Thu, Sep 25, 2025 at 7:47 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>
>>> On Thu, Sep 25, 2025 at 5:59 PM Mario Limonciello (AMD)
>>> <superm1@kernel.org> wrote:
>>>>
>>>> Hybrid sleep will hibernate the system followed by running through
>>>> the suspend routine.  Since both the hibernate and the suspend routine
>>>> will call pm_restrict_gfp_mask(), pm_restore_gfp_mask() must be called
>>>> before starting the suspend sequence.
>>>>
>>>> Add an explicit call to pm_restore_gfp_mask() to power_down() before
>>>> the suspend sequence starts. Don't call pm_restore_gfp_mask() when
>>>> exiting suspend sequence it is already called:
>>>>
>>>> ```
>>>> power_down()
>>>> ->suspend_devices_and_enter()
>>>> -->dpm_resume_end()
>>>> ```
>>>>
>>>> Reported-by: Ionut Nechita <ionut_n2001@yahoo.com>
>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4573
>>>> Tested-by: Ionut Nechita <ionut_n2001@yahoo.com>
>>>> Fixes: 12ffc3b1513eb ("PM: Restrict swap use to later in the suspend sequence")
>>>> Tested-by: Kenneth Crudup <kenny@panix.com>
>>>> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>>> ---
>>>> v2:
>>>>   * Move under CONFIG_SUSPEND scope (LKP robot)
>>>>   * Add tags
>>>> ---
>>>>   kernel/power/hibernate.c | 11 ++++++++++-
>>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>>>> index 2f66ab4538231..52c1818749724 100644
>>>> --- a/kernel/power/hibernate.c
>>>> +++ b/kernel/power/hibernate.c
>>>> @@ -695,6 +695,7 @@ static void power_down(void)
>>>>
>>>>   #ifdef CONFIG_SUSPEND
>>>>          if (hibernation_mode == HIBERNATION_SUSPEND) {
>>>> +               pm_restore_gfp_mask();
>>>>                  error = suspend_devices_and_enter(mem_sleep_current);
>>>>                  if (error) {
>>>>                          hibernation_mode = hibernation_ops ?
>>>> @@ -862,7 +863,15 @@ int hibernate(void)
>>>>                                  power_down();
>>>>                  }
>>>>                  in_suspend = 0;
>>>> -               pm_restore_gfp_mask();
>>>> +               switch (hibernation_mode) {
>>>> +#ifdef CONFIG_SUSPEND
>>>> +               case HIBERNATION_SUSPEND:
>>>> +                       break;
>>>> +#endif
>>>> +               default:
>>>> +                       pm_restore_gfp_mask();
>>>> +                       break;
>>>> +               }
>>>
>>> You're breaking HIBERNATION_TEST_RESUME here AFAICS
>>
>> Well, not really because of the hibernation_mode check.
>>
>>> and power_down() doesn't return.
>>
>> But this still is true.
> 
> Well, except when it does HIBERNATION_SUSPEND.
> 
> But can you just make power_down() call pm_restrict_gfp_mask() before
> returning and leave the code in hibernate() as is?

Ah good suggestion.  I believe so.  I'll test it and get out updated 
patches if that works.