Re: [PATCH 2/3] s390: Implement arch_do_panic

On 7/27/26 11:10 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] arch_do_panic() is bypassed when panic_timeout != 0
> - [Low] Unused declaration of s390_do_panic()
> --
> 
> --- Patch [2]: [PATCH 2/3] s390: Implement arch_do_panic ---
> commit 640e72aef0a165dacc87c9e5913aa8f9442aa6e9
> Author: Mete Durlu <[email protected]>
> 
> s390: Implement arch_do_panic
> 
> This commit implements the arch_do_panic() hook for s390 to handle
> architecture-specific panic actions instead of relying on the panic_notifier
> chain. This change allows user-specified panic_timeout values to be evaluated
> in common code before executing the s390 specific actions.
> 
>> diff --git a/arch/s390/include/asm/ipl.h b/arch/s390/include/asm/ipl.h
>> index b0d00032479d6..38924c4a5d1d7 100644
>> --- a/arch/s390/include/asm/ipl.h
>> +++ b/arch/s390/include/asm/ipl.h
>> @@ -166,5 +166,6 @@ enum diag308_rc {
>>   extern int diag308(unsigned long subcode, void *addr);
>>   extern void store_status(void (*fn)(void *), void *data);
>>   extern void lgr_info_log(void);
>> +void s390_do_panic(void);
> 
> [Severity: Low]
> Is this declaration necessary?
> 
> It looks like s390_do_panic() is declared here but never implemented or used,
> as the actual implementation provided in arch/s390/kernel/ipl.c is named
> arch_do_panic().

This is leftover code. I'll remove it in the next version.

> [ ... ]
> 
>> diff --git a/kernel/panic.c b/kernel/panic.c
>> index 1eb0cdc159d94..de0bda946cabb 100644
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>> @@ -755,9 +755,6 @@ void vpanic(const char *fmt, va_list args)
>>   		pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
>>   			 "twice on console to return to the boot prom\n");
>>   	}
>> -#endif
>> -#if defined(CONFIG_S390)
>> -	disabled_wait();
>>   #endif
>>   	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> 
> [Severity: High]
> Does this change unintentionally bypass the s390 panic actions when
> panic_timeout != 0?
> 
> Looking at vpanic() in kernel/panic.c, if a panic timeout is set, the system
> will restart before reaching arch_do_panic():
> 
>      if (panic_timeout != 0) {
>          ...
>          emergency_restart();
>      }
>      pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> 
>      arch_do_panic();
> 
> Since emergency_restart() does not return, will the user-configured on_panic
> actions be lost in this scenario?

Yes, this is the intended behavior with this change. I'll document it in
the commit message in next version.