From: "Guilherme G. Piccoli" <gpiccoli@igalia.com> To: Petr Mladek <pmladek@suse.com>, Baoquan He <bhe@redhat.com> Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org, kernel@gpiccoli.net, senozhatsky@chromium.org, rostedt@goodmis.org, john.ogness@linutronix.de, feng.tang@intel.com, kexec@lists.infradead.org, dyoung@redhat.com, keescook@chromium.org, anton@enomsg.org, ccross@android.com, tony.luck@intel.com Subject: Re: [PATCH V3] panic: Move panic_print before kmsg dumpers Date: Wed, 19 Jan 2022 13:03:15 -0300 [thread overview] Message-ID: <94bb12a2-a788-cee6-7d4f-dc0ac581fb39@igalia.com> (raw) In-Reply-To: <YegytkfED+QI56Y8@alley> Thanks again Petr, for the deep analysis! Much appreciated. Some comments inline below: On 19/01/2022 12:48, Petr Mladek wrote: >[...] > From my POV, the function of panic notifiers is not well defined. They > do various things, for example: > [...] > > The do more that just providing information. Some are risky. It is not > easy to disable a particular one. We are trying to change that here: https://lore.kernel.org/lkml/20220108153451.195121-1-gpiccoli@igalia.com/ Your review/comments are very welcome =) > [...] > It might make sense to allow to call kmsg_dump before panic notifiers > to reduce the risk of a breakage. But I do not have enough experience > with them to judge this. > > I can't remember any bug report in this code. I guess that only > few people use kmsg_dump. One of the problems doing that is that RCU and hung task detector, for example, have panic notifiers to disable themselves in the panic scenario - if we kmsg_dump() _before_ the panic notifiers, we may have intermixed messages, all messy...so for me it makes sense to keep the kmsg_dump() after panic notifiers. > [...] > Yes, panic_print_sys_info() increases the risk that the crash dump > will not succeed. But the change makes sense because: > > + panic_print_sys_info() does nothing by default. Most users will > not enable it together with crash dump. > > + Guilherme uses crash dump only to dump the kernel log. It might > be more reliable than kmsg_dump. In this case, panic_print_sys_info() > is the only way to get the extra information. > > + panic_print_sys_info() might be useful even with full crash dump. > For example, ftrace messages are not easy to read from the memory > dump. The last point is really good, I didn't consider that before but makes a lot of sense - we can now dump (a hopefully small!) ftrace/event trace buffer to dmesg before a kdump, making it pretty easy to read that later. Cheers, Guilherme
WARNING: multiple messages have this Message-ID (diff)
From: Guilherme G. Piccoli <gpiccoli@igalia.com> To: kexec@lists.infradead.org Subject: [PATCH V3] panic: Move panic_print before kmsg dumpers Date: Wed, 19 Jan 2022 13:03:15 -0300 [thread overview] Message-ID: <94bb12a2-a788-cee6-7d4f-dc0ac581fb39@igalia.com> (raw) In-Reply-To: <YegytkfED+QI56Y8@alley> Thanks again Petr, for the deep analysis! Much appreciated. Some comments inline below: On 19/01/2022 12:48, Petr Mladek wrote: >[...] > From my POV, the function of panic notifiers is not well defined. They > do various things, for example: > [...] > > The do more that just providing information. Some are risky. It is not > easy to disable a particular one. We are trying to change that here: https://lore.kernel.org/lkml/20220108153451.195121-1-gpiccoli at igalia.com/ Your review/comments are very welcome =) > [...] > It might make sense to allow to call kmsg_dump before panic notifiers > to reduce the risk of a breakage. But I do not have enough experience > with them to judge this. > > I can't remember any bug report in this code. I guess that only > few people use kmsg_dump. One of the problems doing that is that RCU and hung task detector, for example, have panic notifiers to disable themselves in the panic scenario - if we kmsg_dump() _before_ the panic notifiers, we may have intermixed messages, all messy...so for me it makes sense to keep the kmsg_dump() after panic notifiers. > [...] > Yes, panic_print_sys_info() increases the risk that the crash dump > will not succeed. But the change makes sense because: > > + panic_print_sys_info() does nothing by default. Most users will > not enable it together with crash dump. > > + Guilherme uses crash dump only to dump the kernel log. It might > be more reliable than kmsg_dump. In this case, panic_print_sys_info() > is the only way to get the extra information. > > + panic_print_sys_info() might be useful even with full crash dump. > For example, ftrace messages are not easy to read from the memory > dump. The last point is really good, I didn't consider that before but makes a lot of sense - we can now dump (a hopefully small!) ftrace/event trace buffer to dmesg before a kdump, making it pretty easy to read that later. Cheers, Guilherme
next prev parent reply other threads:[~2022-01-19 16:03 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-14 18:30 [PATCH V3] panic: Move panic_print before kmsg dumpers Guilherme G. Piccoli 2022-01-14 18:30 ` Guilherme G. Piccoli 2022-01-17 3:33 ` Baoquan He 2022-01-17 3:33 ` Baoquan He 2022-01-17 6:13 ` Baoquan He 2022-01-17 6:13 ` Baoquan He 2022-01-17 12:58 ` Guilherme G. Piccoli 2022-01-17 12:58 ` Guilherme G. Piccoli 2022-01-19 7:13 ` Baoquan He 2022-01-19 7:13 ` Baoquan He 2022-01-19 12:57 ` Guilherme G. Piccoli 2022-01-19 12:57 ` Guilherme G. Piccoli 2022-01-19 15:48 ` Petr Mladek 2022-01-19 15:48 ` Petr Mladek 2022-01-19 16:03 ` Guilherme G. Piccoli [this message] 2022-01-19 16:03 ` Guilherme G. Piccoli 2022-01-20 9:39 ` Petr Mladek 2022-01-20 9:39 ` Petr Mladek 2022-01-20 15:51 ` Guilherme G. Piccoli 2022-01-20 15:51 ` Guilherme G. Piccoli 2022-01-20 8:51 ` Baoquan He 2022-01-20 8:51 ` Baoquan He 2022-01-20 21:36 ` Guilherme G. Piccoli 2022-01-20 21:36 ` Guilherme G. Piccoli 2022-01-21 2:31 ` Baoquan He 2022-01-21 2:31 ` Baoquan He 2022-01-21 13:17 ` Guilherme G. Piccoli 2022-01-21 13:17 ` Guilherme G. Piccoli 2022-01-22 10:31 ` Baoquan He 2022-01-22 10:31 ` Baoquan He 2022-01-22 13:49 ` Guilherme G. Piccoli 2022-01-22 13:49 ` Guilherme G. Piccoli 2022-01-26 3:29 ` Baoquan He 2022-01-26 3:29 ` Baoquan He 2022-01-21 15:00 ` Michael Kelley (LINUX) 2022-01-21 15:00 ` Michael Kelley 2022-01-22 4:33 ` Baoquan He 2022-01-22 4:33 ` Baoquan He 2022-01-24 16:57 ` Michael Kelley (LINUX) 2022-01-24 16:57 ` Michael Kelley 2022-01-26 11:51 ` Petr Mladek 2022-01-26 11:51 ` Petr Mladek 2022-01-29 8:00 ` Baoquan He 2022-01-29 8:00 ` Baoquan He 2022-02-02 17:43 ` Michael Kelley (LINUX) 2022-02-02 17:43 ` Michael Kelley 2022-02-07 8:33 ` Baoquan He 2022-02-07 8:33 ` Baoquan He 2022-01-28 9:03 ` Baoquan He 2022-01-28 9:03 ` Baoquan He 2022-01-28 18:24 ` Michael Kelley (LINUX) 2022-01-28 18:24 ` Michael Kelley 2022-01-29 7:42 ` Baoquan He 2022-01-29 7:42 ` Baoquan He 2022-01-19 18:38 ` Petr Mladek 2022-01-19 18:38 ` Petr Mladek 2022-01-19 19:51 ` Guilherme G. Piccoli 2022-01-19 19:51 ` Guilherme G. Piccoli
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=94bb12a2-a788-cee6-7d4f-dc0ac581fb39@igalia.com \ --to=gpiccoli@igalia.com \ --cc=akpm@linux-foundation.org \ --cc=anton@enomsg.org \ --cc=bhe@redhat.com \ --cc=ccross@android.com \ --cc=dyoung@redhat.com \ --cc=feng.tang@intel.com \ --cc=john.ogness@linutronix.de \ --cc=keescook@chromium.org \ --cc=kernel@gpiccoli.net \ --cc=kexec@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=pmladek@suse.com \ --cc=rostedt@goodmis.org \ --cc=senozhatsky@chromium.org \ --cc=tony.luck@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.