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

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

[Shivang Upadhyay]

fadump is registered in panic_notifier_list and gets triggered
before kmsg_dump_desc() in the panic path. As a result,
kmsg_dumpers (e.g. pstore) are not executed.

Invoke kmsg_dump_desc() from the fadump handler to ensure
kmsg_dumpers are called during panic.

Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reported-by: Shirisha G <shirisha@linux.ibm.com>
Suggested-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 arch/powerpc/kernel/setup-common.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index b1761909c23f..c329b071643d 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"
 
@@ -748,6 +749,8 @@ static int ppc_panic_fadump_handler(struct notifier_block *this,
 	 * If firmware-assisted dump has been registered then trigger
 	 * its callback and let the firmware handles everything else.
 	 */
+	kmsg_dump_desc(KMSG_DUMP_PANIC, (char *)ptr);
+
 	crash_fadump(NULL, ptr);
 
 	return NOTIFY_DONE;
-- 
2.53.0

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

[Sourabh Jain]

Hello Shivang,

On 07/04/26 11:13, 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 (e.g. pstore) are not executed.

Can you emphasize why it is important to call kmsg_dump_desc()?
Because fadump captures the full memory dump anyway.


> Invoke kmsg_dump_desc() from the fadump handler to ensure
> kmsg_dumpers are called during panic.

Can you please add some command output from before and after
this patch series? It will make it easier to understand the impact of
the changes introduced.


>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Reported-by: Shirisha G <shirisha@linux.ibm.com>
> Suggested-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>   arch/powerpc/kernel/setup-common.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index b1761909c23f..c329b071643d 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"
>   
> @@ -748,6 +749,8 @@ static int ppc_panic_fadump_handler(struct notifier_block *this,
>   	 * If firmware-assisted dump has been registered then trigger
>   	 * its callback and let the firmware handles everything else.
>   	 */
> +	kmsg_dump_desc(KMSG_DUMP_PANIC, (char *)ptr);

Can you move the kmsg_dump_desc() function before the comment?
The comment was for crash_fadump() function.

Yes, panic() passes the reason for the panic as a string via ptr. So it 
is good
to pass the same to the message dumpers.

As sashiko suggested:
https://sashiko.dev/#/patchset/20260407054341.308710-1-shivangu%40linux.ibm.com 


I think we should call kmsg_dump_desc() only when fadump is registered, 
to avoid calling
it multiple times. If fadump is not registered, this can be called twice 
in the panic path,
first here and then again in the vpanic() function. Checkout 
should_fadump_crash() function.

The second point from Sashiko is about introducing something like 
crash_kexec_post_notifiers
to give users the option to call message dumpers on the vpanic path, or 
on the other two
paths as well (system reset and die) when fadump is configured.

I think this is not really needed because there are already existing 
crash paths hitting crash_fadump
(system reset and die) on which kmsg_dump_desc() is called directly.

> +
>   	crash_fadump(NULL, ptr);
>

As of now, crash_fadump() (which initiates memory dump
collection) is called through three different paths:

1. System Reset
2. die/oops
3. panic

We call kmsg_dump() -> kmsg_dump_desc() to invoke all message
dumpers on the system reset and die/oops paths. The only
remaining path where kmsg_dump_desc() is not called is the
panic path.

panic() -> vpanic() -> panic notifier call chain -> crash_fadump()

I think it is good to fix this path as well. Thanks for the fix.

- Sourabh Jain

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

[Shivang Upadhyay]

Hi Sourabh,

On Tue, 2026-04-07 at 21:00 +0530, Sourabh Jain wrote:
> Hello Shivang,
> 
> On 07/04/26 11:13, 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 (e.g. pstore) are not executed.
> 
> Can you emphasize why it is important to call kmsg_dump_desc()?
> Because fadump captures the full memory dump anyway.
> 

Sure. So pstore kmsg_dump can be useful in the cases where we fail to
reboot system with kdump/fadump. Because pstore kmsg dump is collected
during the crashing kernel, it can be *only* information that we
collect from that system.

> 
> > Invoke kmsg_dump_desc() from the fadump handler to ensure
> > kmsg_dumpers are called during panic.
> 
> Can you please add some command output from before and after
> this patch series? It will make it easier to understand the impact of
> the changes 

To test for this case, we just made sure that pstore kmsg-dump file's
time stamp was getting updated or not. Currently in fadump's crash path
pstore isn't collecting any logs.

> 
> > 
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> > Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Reported-by: Shirisha G <shirisha@linux.ibm.com>
> > Suggested-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> > Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> > ---
> >   arch/powerpc/kernel/setup-common.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/setup-common.c
> > b/arch/powerpc/kernel/setup-common.c
> > index b1761909c23f..c329b071643d 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"
> >   
> > @@ -748,6 +749,8 @@ static int ppc_panic_fadump_handler(struct
> > notifier_block *this,
> >   	 * If firmware-assisted dump has been registered then
> > trigger
> >   	 * its callback and let the firmware handles everything
> > else.
> >   	 */
> > +	kmsg_dump_desc(KMSG_DUMP_PANIC, (char *)ptr);
> 
> Can you move the kmsg_dump_desc() function before the comment?
> The comment was for crash_fadump() function.
> 
Ack.
> Yes, panic() passes the reason for the panic as a string via ptr. So
> it 
> is good
> to pass the same to the message dumpers.
> 
> As sashiko suggested:
> https://sashiko.dev/#/patchset/20260407054341.308710-1-shivangu%40linux.ibm.com
>  
> 
> 
> I think we should call kmsg_dump_desc() only when fadump is
> registered, 
> to avoid calling
> it multiple times. If fadump is not registered, this can be called
> twice 
> in the panic path,
> first here and then again in the vpanic() function. Checkout 
> should_fadump_crash() function.
> 

Sure, ill find a new place to call this.

> The second point from Sashiko is about introducing something like 
> crash_kexec_post_notifiers
> to give users the option to call message dumpers on the vpanic path,
> or 
> on the other two
> paths as well (system reset and die) when fadump is configured.
> 
> I think this is not really needed because there are already existing 
> crash paths hitting crash_fadump
> (system reset and die) on which kmsg_dump_desc() is called directly.
> 
> > +
> >   	crash_fadump(NULL, ptr);
> > 
> 
> As of now, crash_fadump() (which initiates memory dump
> collection) is called through three different paths:
> 
> 1. System Reset
> 2. die/oops
> 3. panic
> 
> We call kmsg_dump() -> kmsg_dump_desc() to invoke all message
> dumpers on the system reset and die/oops paths. The only
> remaining path where kmsg_dump_desc() is not called is the
> panic path.
> 
> panic() -> vpanic() -> panic notifier call chain -> crash_fadump()
> 

Yeah, the other paths should be covered by this line in crash path here
[1].

> I think it is good to fix this path as well. Thanks for the fix.
> 
> - Sourabh Jain


Thanks,
~Shivang.

[1] http://elixir.bootlin.com/linux/v6.15.6/source/kernel/panic.c#L387