* [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
@ 2025-07-12 3:08 Zihuan Zhang
2025-07-14 8:44 ` Zihuan Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Zihuan Zhang @ 2025-07-12 3:08 UTC (permalink / raw)
To: Rafael J . Wysocki, Christian Brauner
Cc: Len Brown, Pavel Machek, linux-pm, linux-kernel, 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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-12 3:08 [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling Zihuan Zhang
@ 2025-07-14 8:44 ` Zihuan Zhang
2025-07-14 17:57 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Zihuan Zhang @ 2025-07-14 8:44 UTC (permalink / raw)
To: Rafael J . Wysocki, Christian Brauner
Cc: Len Brown, Pavel Machek, linux-pm, linux-kernel
Hi Rafael,
Just a gentle ping on this patch.
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.
在 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-14 8:44 ` Zihuan Zhang
@ 2025-07-14 17:57 ` Rafael J. Wysocki
2025-07-15 6:12 ` Zihuan Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2025-07-14 17:57 UTC (permalink / raw)
To: Zihuan Zhang
Cc: Rafael J . Wysocki, Christian Brauner, Len Brown, Pavel Machek,
linux-pm, linux-kernel
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!
> 在 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-14 17:57 ` Rafael J. Wysocki
@ 2025-07-15 6:12 ` Zihuan Zhang
2025-07-15 12:48 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Zihuan Zhang @ 2025-07-15 6:12 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Christian Brauner, Len Brown, Pavel Machek, linux-pm,
linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-15 6:12 ` Zihuan Zhang
@ 2025-07-15 12:48 ` Rafael J. Wysocki
2025-07-16 2:04 ` Zihuan Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2025-07-15 12:48 UTC (permalink / raw)
To: Zihuan Zhang
Cc: Rafael J. Wysocki, Christian Brauner, Len Brown, Pavel Machek,
linux-pm, linux-kernel
On Tue, Jul 15, 2025 at 8:12 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>
> 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?
Possibly, if this overhead is significant, but is it?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-15 12:48 ` Rafael J. Wysocki
@ 2025-07-16 2:04 ` Zihuan Zhang
2025-07-16 12:23 ` Rafael J. Wysocki
0 siblings, 1 reply; 8+ messages in thread
From: Zihuan Zhang @ 2025-07-16 2:04 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Christian Brauner, Len Brown, Pavel Machek, linux-pm,
linux-kernel
在 2025/7/15 20:48, Rafael J. Wysocki 写道:
> On Tue, Jul 15, 2025 at 8:12 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>> 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?
> Possibly, if this overhead is significant, but is it?
We've done some testing using ftrace to measure the overhead of
filesystems_thaw(). When freeze_filesystems is not enabled, the overhead
is typically around 15–40 microseconds.
However, when freeze is enabled, we observed that filesystems_thaw() can
take over 3 seconds to complete (e.g., 3,450,644 us in one test case).
freeze_filesystems not enabled:
# tracer: function_graph
#
# CPU DURATION FUNCTION CALLS
# | | | | | | |
4) + 15.740 us | filesystems_thaw();
11) + 16.894 us | filesystems_thaw();
10) + 17.805 us | filesystems_thaw();
8) + 37.762 us | filesystems_thaw();
------------------------------------------
11) systemd-54512 => systemd-66433
------------------------------------------
11) + 15.167 us | filesystems_thaw();
6) + 16.760 us | filesystems_thaw();
7) + 14.870 us | filesystems_thaw();
3) + 16.171 us | filesystems_thaw();
1) + 16.461 us | filesystems_thaw();
------------------------------------------
3) systemd-71984 => systemd-73036
------------------------------------------
3) + 28.314 us | filesystems_thaw();
freeze_filesystems enabled:
10) | filesystems_thaw() {
2) $ 3450644 us | } /* filesystems_thaw */
------------------------------------------
1) systemd-72561 => systemd-99210
------------------------------------------
1) | filesystems_thaw() {
------------------------------------------
7) systemd-71501 => systemd-99210
------------------------------------------
7) $ 3429306 us | } /* filesystems_thaw */
------------------------------------------
7) systemd-99210 => systemd-100028
------------------------------------------
7) | filesystems_thaw() {
------------------------------------------
4) systemd-53278 => systemd-100028
------------------------------------------
4) $ 3270122 us | } /* filesystems_thaw */
------------------------------------------
7) systemd-100028 => systemd-100720
------------------------------------------
7) $ 3446496 us | filesystems_thaw();
------------------------------------------
7) systemd-100720 => systemd-112075
------------------------------------------
7) | filesystems_thaw() {
------------------------------------------
11) systemd-66433 => systemd-112075
------------------------------------------
11) $ 3454117 us | } /* filesystems_thaw */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-16 2:04 ` Zihuan Zhang
@ 2025-07-16 12:23 ` Rafael J. Wysocki
2025-07-17 0:37 ` Zihuan Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2025-07-16 12:23 UTC (permalink / raw)
To: Zihuan Zhang
Cc: Rafael J. Wysocki, Christian Brauner, Len Brown, Pavel Machek,
linux-pm, linux-kernel
On Wed, Jul 16, 2025 at 4:04 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>
>
> 在 2025/7/15 20:48, Rafael J. Wysocki 写道:
> > On Tue, Jul 15, 2025 at 8:12 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
> >> 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?
> > Possibly, if this overhead is significant, but is it?
>
> We've done some testing using ftrace to measure the overhead of
> filesystems_thaw(). When freeze_filesystems is not enabled, the overhead
> is typically around 15–40 microseconds.
So this is the time that can be saved by adding a
filesystem_freeze_enabled check before calling filesystems_thaw()
IIUC.
I'd say don't bother.
> However, when freeze is enabled, we observed that filesystems_thaw() can
> take over 3 seconds to complete (e.g., 3,450,644 us in one test case).
>
> freeze_filesystems not enabled:
>
> # tracer: function_graph
> #
> # CPU DURATION FUNCTION CALLS
> # | | | | | | |
> 4) + 15.740 us | filesystems_thaw();
> 11) + 16.894 us | filesystems_thaw();
> 10) + 17.805 us | filesystems_thaw();
> 8) + 37.762 us | filesystems_thaw();
> ------------------------------------------
> 11) systemd-54512 => systemd-66433
> ------------------------------------------
>
> 11) + 15.167 us | filesystems_thaw();
> 6) + 16.760 us | filesystems_thaw();
> 7) + 14.870 us | filesystems_thaw();
> 3) + 16.171 us | filesystems_thaw();
> 1) + 16.461 us | filesystems_thaw();
> ------------------------------------------
> 3) systemd-71984 => systemd-73036
> ------------------------------------------
>
> 3) + 28.314 us | filesystems_thaw();
>
> freeze_filesystems enabled:
>
> 10) | filesystems_thaw() {
> 2) $ 3450644 us | } /* filesystems_thaw */
> ------------------------------------------
> 1) systemd-72561 => systemd-99210
> ------------------------------------------
>
> 1) | filesystems_thaw() {
> ------------------------------------------
> 7) systemd-71501 => systemd-99210
> ------------------------------------------
>
> 7) $ 3429306 us | } /* filesystems_thaw */
> ------------------------------------------
> 7) systemd-99210 => systemd-100028
> ------------------------------------------
>
> 7) | filesystems_thaw() {
> ------------------------------------------
> 4) systemd-53278 => systemd-100028
> ------------------------------------------
>
> 4) $ 3270122 us | } /* filesystems_thaw */
> ------------------------------------------
> 7) systemd-100028 => systemd-100720
> ------------------------------------------
>
> 7) $ 3446496 us | filesystems_thaw();
> ------------------------------------------
> 7) systemd-100720 => systemd-112075
> ------------------------------------------
>
> 7) | filesystems_thaw() {
> ------------------------------------------
> 11) systemd-66433 => systemd-112075
> ------------------------------------------
>
> 11) $ 3454117 us | } /* filesystems_thaw */
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling
2025-07-16 12:23 ` Rafael J. Wysocki
@ 2025-07-17 0:37 ` Zihuan Zhang
0 siblings, 0 replies; 8+ messages in thread
From: Zihuan Zhang @ 2025-07-17 0:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Christian Brauner, Len Brown, Pavel Machek, linux-pm,
linux-kernel
在 2025/7/16 20:23, Rafael J. Wysocki 写道:
> On Wed, Jul 16, 2025 at 4:04 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>>
>> 在 2025/7/15 20:48, Rafael J. Wysocki 写道:
>>> On Tue, Jul 15, 2025 at 8:12 AM Zihuan Zhang <zhangzihuan@kylinos.cn> wrote:
>>>> 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?
>>> Possibly, if this overhead is significant, but is it?
>> We've done some testing using ftrace to measure the overhead of
>> filesystems_thaw(). When freeze_filesystems is not enabled, the overhead
>> is typically around 15–40 microseconds.
> So this is the time that can be saved by adding a
> filesystem_freeze_enabled check before calling filesystems_thaw()
> IIUC.
>
> I'd say don't bother.
>
Understood, thanks!
>> However, when freeze is enabled, we observed that filesystems_thaw() can
>> take over 3 seconds to complete (e.g., 3,450,644 us in one test case).
>>
>> freeze_filesystems not enabled:
>>
>> # tracer: function_graph
>> #
>> # CPU DURATION FUNCTION CALLS
>> # | | | | | | |
>> 4) + 15.740 us | filesystems_thaw();
>> 11) + 16.894 us | filesystems_thaw();
>> 10) + 17.805 us | filesystems_thaw();
>> 8) + 37.762 us | filesystems_thaw();
>> ------------------------------------------
>> 11) systemd-54512 => systemd-66433
>> ------------------------------------------
>>
>> 11) + 15.167 us | filesystems_thaw();
>> 6) + 16.760 us | filesystems_thaw();
>> 7) + 14.870 us | filesystems_thaw();
>> 3) + 16.171 us | filesystems_thaw();
>> 1) + 16.461 us | filesystems_thaw();
>> ------------------------------------------
>> 3) systemd-71984 => systemd-73036
>> ------------------------------------------
>>
>> 3) + 28.314 us | filesystems_thaw();
>>
>> freeze_filesystems enabled:
>>
>> 10) | filesystems_thaw() {
>> 2) $ 3450644 us | } /* filesystems_thaw */
>> ------------------------------------------
>> 1) systemd-72561 => systemd-99210
>> ------------------------------------------
>>
>> 1) | filesystems_thaw() {
>> ------------------------------------------
>> 7) systemd-71501 => systemd-99210
>> ------------------------------------------
>>
>> 7) $ 3429306 us | } /* filesystems_thaw */
>> ------------------------------------------
>> 7) systemd-99210 => systemd-100028
>> ------------------------------------------
>>
>> 7) | filesystems_thaw() {
>> ------------------------------------------
>> 4) systemd-53278 => systemd-100028
>> ------------------------------------------
>>
>> 4) $ 3270122 us | } /* filesystems_thaw */
>> ------------------------------------------
>> 7) systemd-100028 => systemd-100720
>> ------------------------------------------
>>
>> 7) $ 3446496 us | filesystems_thaw();
>> ------------------------------------------
>> 7) systemd-100720 => systemd-112075
>> ------------------------------------------
>>
>> 7) | filesystems_thaw() {
>> ------------------------------------------
>> 11) systemd-66433 => systemd-112075
>> ------------------------------------------
>>
>> 11) $ 3454117 us | } /* filesystems_thaw */
>>
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-17 0:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-12 3:08 [PATCH v1] PM: suspend: clean up redundant filesystems_freeze/thaw handling Zihuan Zhang
2025-07-14 8:44 ` Zihuan Zhang
2025-07-14 17:57 ` Rafael J. Wysocki
2025-07-15 6:12 ` Zihuan Zhang
2025-07-15 12:48 ` Rafael J. Wysocki
2025-07-16 2:04 ` Zihuan Zhang
2025-07-16 12:23 ` Rafael J. Wysocki
2025-07-17 0:37 ` Zihuan Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).