Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling

[Zihuan Zhang <zhangzihuan@kylinos.cn>]

Hi Rafael,

在 2025/7/15 01:57, Rafael J. Wysocki 写道:
> Hi,
>
> On Mon, Jul 14, 2025 at 10:44 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>> Hi Rafael,
>>
>> Just a gentle ping on this patch.
> I've lost track of it for some reason, sorry.
>
>> I realized I forgot to mention an important motivation in the changelog:
>> calling filesystems_freeze() twice (from both suspend_prepare() and
>> enter_state()) lead to a black screen and make the system unable to resume..
>>
>> This patch avoids the duplicate call and resolves that issue.
> Now applied as a fix for 6.16-rc7, thank you!


Thanks for the reply!

Just a quick follow-up question — we noticed that even when the “freeze 
filesystems” feature is not enabled, the current code still calls 
filesystems_thaw().

Do you think it would make sense to guard this with a static key (or 
another mechanism) to avoid unnecessary overhead?



>> 在 2025/7/12 11:08, Zihuan Zhang 写道:
>>> The recently introduced support for freezing filesystems during system
>>> suspend included calls to filesystems_freeze() in both suspend_prepare()
>>> and enter_state(), as well as calls to filesystems_thaw() in both
>>> suspend_finish() and the Unlock path in enter_state(). These are
>>> redundant.
>>>
>>> - filesystems_freeze() is already called in suspend_prepare(), which is
>>>     the proper and consistent place to handle pre-suspend operations. The
>>> second call in enter_state() is unnecessary and removed.
>>>
>>> - filesystems_thaw() is invoked in suspend_finish(), which covers
>>>     successful suspend/resume paths. In the failure case , we add a call
>>> to filesystems_thaw() only when needed, avoiding the duplicate call in
>>> the general Unlock path.
>>>
>>> This change simplifies the suspend code and avoids repeated freeze/thaw
>>> calls, while preserving correct ordering and behavior.
>>>
>>> Fixes: eacfbf74196f91e4c26d9f8c78e1576c1225cd8c ("power: freeze filesystems during suspend/resume")
>>> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
>>> ---
>>>    kernel/power/suspend.c | 4 +---
>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>>> index bb608b68fb30..8f3e4c48d5cd 100644
>>> --- a/kernel/power/suspend.c
>>> +++ b/kernel/power/suspend.c
>>> @@ -384,6 +384,7 @@ static int suspend_prepare(suspend_state_t state)
>>>                return 0;
>>>
>>>        dpm_save_failed_step(SUSPEND_FREEZE);
>>> +     filesystems_thaw();
>>>        pm_notifier_call_chain(PM_POST_SUSPEND);
>>>     Restore:
>>>        pm_restore_console();
>>> @@ -593,8 +594,6 @@ static int enter_state(suspend_state_t state)
>>>                ksys_sync_helper();
>>>                trace_suspend_resume(TPS("sync_filesystems"), 0, false);
>>>        }
>>> -     if (filesystem_freeze_enabled)
>>> -             filesystems_freeze();
>>>
>>>        pm_pr_dbg("Preparing system for sleep (%s)\n", mem_sleep_labels[state]);
>>>        pm_suspend_clear_flags();
>>> @@ -614,7 +613,6 @@ static int enter_state(suspend_state_t state)
>>>        pm_pr_dbg("Finishing wakeup.\n");
>>>        suspend_finish();
>>>     Unlock:
>>> -     filesystems_thaw();
>>>        mutex_unlock(&system_transition_mutex);
>>>        return error;
>>>    }
>> Thanks,
>> Zihuan Zhang
Best regards,
Zihuan Zhang