All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Brennan <stephen.s.brennan@oracle.com>
To: "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
	kexec@lists.infradead.org, linux-doc@vger.kernel.org
Cc: bhe@redhat.com, corbet@lwn.net, kernel@gpiccoli.net,
	linux-kernel@vger.kernel.org, kernel-dev@igalia.com,
	dyoung@redhat.com, vgoyal@redhat.com,
	linux-debuggers@vger.kernel.org
Subject: Re: [PATCH] Documentation: Improve crash_kexec_post_notifiers description
Date: Fri, 30 Aug 2024 10:15:50 -0700	[thread overview]
Message-ID: <87ed66q6d5.fsf@oracle.com> (raw)
In-Reply-To: <20240830140401.458542-1-gpiccoli@igalia.com>

"Guilherme G. Piccoli" <gpiccoli@igalia.com> writes:
> Be more clear about the downsides, the upsides (yes, there are some!)
> and about code that unconditionally sets that.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index efc52ddc6864..cb25dc5cbe9a 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -913,12 +913,16 @@
>  			the parameter has no effect.
>  
>  	crash_kexec_post_notifiers
> -			Run kdump after running panic-notifiers and dumping
> -			kmsg. This only for the users who doubt kdump always
> -			succeeds in any situation.
> -			Note that this also increases risks of kdump failure,
> -			because some panic notifiers can make the crashed
> -			kernel more unstable.
> +			Only jump to kdump kernel after running the panic
> +			notifiers and dumping kmsg. This option increases the
> +			risks of a kdump failure, since some panic notifiers
> +			can make the crashed kernel more unstable. As a bright
> +			side, it might allow to collect more data on dmesg like
> +			stack traces from other CPUs or extra data dumped by
> +			panic_print. This is usually only for users who doubt
> +			kdump will succeed every time.

This is definitely clearer and an improvement! But I didn't (and still
don't) love the phrase "users who doubt kdump will succeed" because I
think that implies user error or silly beliefs.

What if these two sentences read something like:

In configurations where kdump may not be reliable, running the panic
notifiers can allow collecting more data on dmesg, like stack traces
from other CPUS or extra data dumped by panic_print.

> Notice that some code
> +			enables this option unconditionally, like Hyper-V,
> +			PowerPC (fadump) and AMD SEV.

Yes, great addition.

With or without my suggestions it's an improvement, so:

Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Brennan <stephen.s.brennan@oracle.com>
To: "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
	kexec@lists.infradead.org, linux-doc@vger.kernel.org
Cc: bhe@redhat.com, vgoyal@redhat.com, dyoung@redhat.com,
	corbet@lwn.net, linux-kernel@vger.kernel.org,
	linux-debuggers@vger.kernel.org, kernel@gpiccoli.net,
	kernel-dev@igalia.com,
	"Guilherme G. Piccoli" <gpiccoli@igalia.com>
Subject: Re: [PATCH] Documentation: Improve crash_kexec_post_notifiers description
Date: Fri, 30 Aug 2024 10:15:50 -0700	[thread overview]
Message-ID: <87ed66q6d5.fsf@oracle.com> (raw)
In-Reply-To: <20240830140401.458542-1-gpiccoli@igalia.com>

"Guilherme G. Piccoli" <gpiccoli@igalia.com> writes:
> Be more clear about the downsides, the upsides (yes, there are some!)
> and about code that unconditionally sets that.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index efc52ddc6864..cb25dc5cbe9a 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -913,12 +913,16 @@
>  			the parameter has no effect.
>  
>  	crash_kexec_post_notifiers
> -			Run kdump after running panic-notifiers and dumping
> -			kmsg. This only for the users who doubt kdump always
> -			succeeds in any situation.
> -			Note that this also increases risks of kdump failure,
> -			because some panic notifiers can make the crashed
> -			kernel more unstable.
> +			Only jump to kdump kernel after running the panic
> +			notifiers and dumping kmsg. This option increases the
> +			risks of a kdump failure, since some panic notifiers
> +			can make the crashed kernel more unstable. As a bright
> +			side, it might allow to collect more data on dmesg like
> +			stack traces from other CPUs or extra data dumped by
> +			panic_print. This is usually only for users who doubt
> +			kdump will succeed every time.

This is definitely clearer and an improvement! But I didn't (and still
don't) love the phrase "users who doubt kdump will succeed" because I
think that implies user error or silly beliefs.

What if these two sentences read something like:

In configurations where kdump may not be reliable, running the panic
notifiers can allow collecting more data on dmesg, like stack traces
from other CPUS or extra data dumped by panic_print.

> Notice that some code
> +			enables this option unconditionally, like Hyper-V,
> +			PowerPC (fadump) and AMD SEV.

Yes, great addition.

With or without my suggestions it's an improvement, so:

Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>

  reply	other threads:[~2024-08-30 17:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-30 14:03 [PATCH] Documentation: Improve crash_kexec_post_notifiers description Guilherme G. Piccoli
2024-08-30 14:03 ` Guilherme G. Piccoli
2024-08-30 17:15 ` Stephen Brennan [this message]
2024-08-30 17:15   ` Stephen Brennan
2024-08-30 18:23   ` Guilherme G. Piccoli
2024-08-30 18:23     ` 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=87ed66q6d5.fsf@oracle.com \
    --to=stephen.s.brennan@oracle.com \
    --cc=bhe@redhat.com \
    --cc=corbet@lwn.net \
    --cc=dyoung@redhat.com \
    --cc=gpiccoli@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=kernel@gpiccoli.net \
    --cc=kexec@lists.infradead.org \
    --cc=linux-debuggers@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vgoyal@redhat.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: link
Be 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.