linux-kernel - Re: [PATCH v1] PM: suspend: clean up redundant filesystems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <65a378b2-887b-43f3-85d8-b689b4c92817@kylinos.cn>
Date: Thu, 17 Jul 2025 08:37:25 +0800
From: Zihuan Zhang <zhangzihuan@...inos.cn>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Christian Brauner <brauner@...nel.org>, Len Brown <len.brown@...el.com>,
 Pavel Machek <pavel@...nel.org>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] PM: suspend: clean up redundant
 filesystems_freeze/thaw handling


在 2025/7/16 20:23, Rafael J. Wysocki 写道:
> On Wed, Jul 16, 2025 at 4:04 AM Zihuan Zhang <zhangzihuan@...inos.cn> wrote:
>>
>> 在 2025/7/15 20:48, Rafael J. Wysocki 写道:
>>> On Tue, Jul 15, 2025 at 8:12 AM Zihuan Zhang <zhangzihuan@...inos.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@...inos.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 */
>>
>>
>>