Re: [PATCH v2] ppc/fadump: invoke kmsg_dump in fadump panic path

[Mahesh J Salgaonkar]

On 2026-04-12 17:00:57 Sun, Shivang Upadhyay wrote:
> fadump is registered in panic_notifier_list and gets triggered before
> kmsg_dump_desc() in the panic path. As a result, kmsg_dumpers such as
> pstore are not executed during fadump crashes.
> 
> This is problematic because pstore provides a critical fallback mechanism
> for crash analysis. When fadump fails to successfully reboot the system
> or capture a dump, pstore logs may be the only available information from
> the crashed kernel. Without invoking kmsg_dump_desc() in the fadump path,
> we lose this valuable diagnostic data.
> 
> Invoke kmsg_dump_desc() from the fadump panic handler, but only when
> fadump is actually registered (checked via should_fadump_crash()). This
> ensures kmsg_dumpers are called without duplicating the call that occurs
> later in panic() when fadump is not active.
> 
> The call is placed before crash_fadump() to ensure logs are captured
> before the system attempts to trigger the firmware-assisted dump.
> 
> Cc: Hari Bathini <hbathini@xxxxxxxxxxxxx>
> Cc: Madhavan Srinivasan <maddy@xxxxxxxxxxxxx>
> Cc: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxx>
> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> Cc: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> Reported-by: Shirisha G <shirisha@xxxxxxxxxxxxx>
> Suggested-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
> Signed-off-by: Shivang Upadhyay <shivangu@xxxxxxxxxxxxx>
> ---
> Changelog:
> 
> v2:
>   address comments from Sourabh, and sashikio-ai
>   added should_fadump_crash()
> 
> v1:
>   https://lore.kernel.org/all/20260407054341.308710-1-shivangu@xxxxxxxxxxxxx/
> ---
>  arch/powerpc/kernel/setup-common.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index b1761909c23f..87afc8003a03 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -68,6 +68,7 @@
>  #include <asm/kasan.h>
>  #include <asm/mce.h>
>  #include <asm/systemcfg.h>
> +#include <linux/kmsg_dump.h>
>  
>  #include "setup.h"
>  
> @@ -744,6 +745,13 @@ static int ppc_panic_fadump_handler(struct notifier_block *this,
>  	 */
>  	hard_irq_disable();
>  
> +	/*
> +	 * Invoke kmsg_dump (e.g., pstore) before crash_fadump() as fadump
> +	 * runs before panic()'s kmsg_dump_desc() call.
> +	 */
> +	if (should_fadump_crash())
> +		kmsg_dump_desc(KMSG_DUMP_PANIC, (char *)ptr);
> +

Looks good to me.

Reviewed-by: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxx>

Thanks,
-Mahesh.
>  	/*
>  	 * If firmware-assisted dump has been registered then trigger
>  	 * its callback and let the firmware handles everything else.
> -- 
> 2.53.0
> 
> 

-- 
Mahesh J Salgaonkar