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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0j4biD2Jd5isUGFmwAva1RJsPDCHNpb1VEjM5vTBrk-jQ@mail.gmail.com>
Date: Wed, 16 Jul 2025 14:23:42 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Zihuan Zhang <zhangzihuan@...inos.cn>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, 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

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.


> 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 */
>
>
>