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 <[email protected]> > > Cc: Madhavan Srinivasan <[email protected]> > > Cc: Mahesh Salgaonkar <[email protected]> > > Cc: Michael Ellerman <[email protected]> > > Cc: Ritesh Harjani (IBM) <[email protected]> > > Reported-by: Shirisha G <[email protected]> > > Suggested-by: Sourabh Jain <[email protected]> > > Signed-off-by: Shivang Upadhyay <[email protected]> > > --- > > 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
