linux-kernel - Re: [RFC PATCH] x86 / hibernate: Eliminate the redundant smp_ops.play

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: <CAJZ5v0iigAB97mGBe6Uvr0v0spjqDKan-0O9XGObt5b4ZBvM7A@mail.gmail.com>
Date: Wed, 26 Feb 2025 18:11:51 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: WangYuli <wangyuli@...ontech.com>
Cc: rafael@...nel.org, pavel@...nel.org, tglx@...utronix.de, mingo@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, zhanjun@...ontech.com, 
	niecheng1@...ontech.com, chenlinxuan@...ontech.com, 
	Huacai Chen <chenhuacai@...ngson.cn>, Wentao Guan <guanwentao@...ontech.com>
Subject: Re: [RFC PATCH] x86 / hibernate: Eliminate the redundant
 smp_ops.play_dead assignment

On Mon, Feb 24, 2025 at 8:45 AM WangYuli <wangyuli@...ontech.com> wrote:
>
> It's unnecessary to re-initialize smp_ops.play_dead to play_dead as
> it naturally goes back to play_dead in the freshly booted kernel upon
> device resume.

Is it really?

And what if freeze_secondary_cpus() returns an error, for one example?

> Suggested-by: Huacai Chen <chenhuacai@...ngson.cn>
> Co-developed-by: Wentao Guan <guanwentao@...ontech.com>
> Signed-off-by: Wentao Guan <guanwentao@...ontech.com>
> Signed-off-by: WangYuli <wangyuli@...ontech.com>
> ---
>  arch/x86/power/cpu.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
> index 63230ff8cf4f..023cf9421467 100644
> --- a/arch/x86/power/cpu.c
> +++ b/arch/x86/power/cpu.c
> @@ -297,9 +297,6 @@ static void __noreturn resume_play_dead(void)
>
>  int hibernate_resume_nonboot_cpu_disable(void)
>  {
> -       void (*play_dead)(void) = smp_ops.play_dead;
> -       int ret;
> -
>         /*
>          * Ensure that MONITOR/MWAIT will not be used in the "play dead" loop
>          * during hibernate image restoration, because it is likely that the
> @@ -316,13 +313,11 @@ int hibernate_resume_nonboot_cpu_disable(void)
>          * resume) sleep afterwards, and the resumed kernel will decide itself
>          * what to do with them.
>          */
> -       ret = cpuhp_smt_enable();
> +       int ret = cpuhp_smt_enable();
>         if (ret)
>                 return ret;
>         smp_ops.play_dead = resume_play_dead;
> -       ret = freeze_secondary_cpus(0);
> -       smp_ops.play_dead = play_dead;
> -       return ret;
> +       return freeze_secondary_cpus(0);
>  }
>  #endif
>
> --
> 2.47.2
>