linux-kernel - Re: [PATCH 3/9] crash_core: use panic_try_start() in crash

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: <aKaHvnxEaXF/fLnW@MiWiFi-R3L-srv>
Date: Thu, 21 Aug 2025 10:43:10 +0800
From: Baoquan He <bhe@...hat.com>
To: Jinchao Wang <wangjinchao600@...il.com>
Cc: pmladek@...e.com, akpm@...ux-foundation.org,
	Vivek Goyal <vgoyal@...hat.com>, Dave Young <dyoung@...hat.com>,
	linux-kernel@...r.kernel.org, feng.tang@...ux.alibaba.com,
	joel.granados@...nel.org, john.ogness@...utronix.de,
	namcao@...utronix.de, sravankumarlpu@...il.com,
	kexec@...ts.infradead.org
Subject: Re: [PATCH 3/9] crash_core: use panic_try_start() in crash_kexec()

On 08/20/25 at 05:14pm, Jinchao Wang wrote:
> crash_kexec() had its own code to exclude
> parallel execution by setting panic_cpu.
> This is already handled by panic_try_start().
> 
> Switch to panic_try_start() to remove the
> duplication and keep the logic consistent.
> 
> Signed-off-by: Jinchao Wang <wangjinchao600@...il.com>

I had to use b4 to grab back the whole patchset, but I can't comment on
other patches, especially the patch 1. 

Firstly, this series looks interesting. It does enhance code
readibility. But I am a vim user, I like open code on this one line of
code wrapping. So leave this to other reviewers to decide if this should
be accepted.

Secondly, the lines of your patch log are too short, it's not convenient
for reading. Can you set your mail writer to change this.

Thirdly, please add people to CC in all patches. I don't know why you
only CC me in patch 3 if the whole patchset is related to crash and
panic.

Thanks
Baoquan

> ---
>  kernel/crash_core.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index a4ef79591eb2..bb38bbaf3a26 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2002-2004 Eric Biederman  <ebiederm@...ssion.com>
>   */
>  
> +#include "linux/panic.h"
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/buildid.h>
> @@ -143,17 +144,7 @@ STACK_FRAME_NON_STANDARD(__crash_kexec);
>  
>  __bpf_kfunc void crash_kexec(struct pt_regs *regs)
>  {
> -	int old_cpu, this_cpu;
> -
> -	/*
> -	 * Only one CPU is allowed to execute the crash_kexec() code as with
> -	 * panic().  Otherwise parallel calls of panic() and crash_kexec()
> -	 * may stop each other.  To exclude them, we use panic_cpu here too.
> -	 */
> -	old_cpu = PANIC_CPU_INVALID;
> -	this_cpu = raw_smp_processor_id();
> -
> -	if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
> +	if (panic_try_start()) {
>  		/* This is the 1st CPU which comes here, so go ahead. */
>  		__crash_kexec(regs);
>  
> @@ -161,7 +152,7 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
>  		 * Reset panic_cpu to allow another panic()/crash_kexec()
>  		 * call.
>  		 */
> -		atomic_set(&panic_cpu, PANIC_CPU_INVALID);
> +		panic_reset();
>  	}
>  }
>  
> -- 
> 2.43.0
>