LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Kees Cook <keescook@chromium.org>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>,
	Anton Vorontsov <anton@enomsg.org>,
	Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Rob Herring <robh+dt@kernel.org>,
	Benson Leung <bleung@chromium.org>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	jmorris@namei.org, sashal@kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 0/6] allow ramoops to collect all kmesg_dump events
Date: Tue, 12 May 2020 15:16:55 +0200
Message-ID: <20200512131655.GE17734@linux-b0ei> (raw)
In-Reply-To: <20200506211523.15077-1-keescook@chromium.org>

On Wed 2020-05-06 14:15:17, Kees Cook wrote:
> Hi!
> 
> This is my stab at rearranging a few things based on Pavel's series. Most
> things remain the same; I just tweaked how defaults are arranged and
> detected and expanded the wording in a few places. Pavel, how does this
> v3 look to you?
> 
> Pavel's original cover letter:
> 
> pstore /mnt/console-ramoops-0 outputs only messages below the console
> loglevel, and our console loglevel is set to 3 due to slowness of
> serial console. Which means only errors and worse types of messages
> are recorded. There is no way to have different log levels for
> different consoles.
> 
> This patch series adds a new option to ramoops: max_reason that enables
> it to collect kmdesg dumps for other reasons beside oops and panics.

I was a bit confused by the above explanation. It talks about two
different numbering schemes:

   + console loglevels: emerg, alert, crit, err, warning, ...
   + dump reason: panic, oops, emerg, restart, halt, poweroff

This difference and also the jump from consoles to ramoops is far from
obvious.

My understanding is the following:

It is not possible to set loglevel per console. The global value must
be set by the slowest one. This prevents seeing all messages even
on fast consoles.

Alternative solution is to dump all messages using ramoops. The
problem is that it currently works only during Oops and panic
situation. This is solved by this patchset.


OK, I personally see this as two separate problems:

   1. Missing support to set loglevel per console.
   2. Missing support to dump messages for other reasons.

I would remove the paragraph about console log levels completely.
It is your reason to use ramoops. But it is not reason to modify
the logic about max_reason.


Now, the max_reason logic makes sense only when all the values
have some ordering. Is this the case?

I see it as two distinct sets:

   + panic, oops, emerg: describe how critical is an error situation
   + restart, halt, poweroff: describe behavior when the system goes down

Let's say that panic is more critical than oops. Is restart more
critical than halt?

If you want the dump during restart. Does it mean that you want it
also during emergency situation?

My fear is that this patchset is going to introduce user interface
(max_reason) with a weird logic. IMHO, max_reason is confusing even
in the code and we should not spread this to users.

Is there any reason why the existing printk.always_kmsg_dump option
is not enough for you?

Best Regards,
Petr

  parent reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-06 21:15 Kees Cook
2020-05-06 21:15 ` [PATCH v3 1/6] printk: honor the max_reason field in kmsg_dumper Kees Cook
2020-05-06 21:15 ` [PATCH v3 2/6] pstore/platform: Pass max_reason to kmesg dump Kees Cook
2020-05-06 21:25   ` Joe Perches
2020-05-06 22:40     ` Kees Cook
2020-05-06 21:15 ` [PATCH v3 3/6] pstore/ram: Refactor DT size parsing Kees Cook
2020-05-07 12:57   ` Pavel Tatashin
2020-05-07 18:04     ` Kees Cook
2020-05-06 21:15 ` [PATCH v3 4/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
2020-05-06 21:17   ` Kees Cook
2020-05-12 23:35   ` Tyler Hicks
2020-05-12 23:57     ` Kees Cook
2020-05-16  2:43     ` Kees Cook
2020-05-06 21:15 ` [PATCH v3 5/6] ramoops: Add max_reason optional field to ramoops DT node Kees Cook
2020-05-06 21:15 ` [PATCH v3 6/6] pstore/ram: Adjust module param permissions to reflect reality Kees Cook
2020-05-12 13:16 ` Petr Mladek [this message]
2020-05-12 14:03   ` [PATCH v3 0/6] allow ramoops to collect all kmesg_dump events Pavel Tatashin
2020-05-12 15:52     ` Petr Mladek
2020-05-12 16:03       ` Steven Rostedt
2020-05-12 16:49       ` Pavel Tatashin
2020-05-12 18:53         ` Kees Cook
2020-05-12 18:45       ` Kees Cook
2020-05-13  7:34         ` Petr Mladek
2020-05-13  7:47           ` Kees Cook
2020-05-13 14:35             ` Pavel Tatashin
2020-05-13 20:15               ` Kees Cook

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=20200512131655.GE17734@linux-b0ei \
    --to=pmladek@suse.com \
    --cc=anton@enomsg.org \
    --cc=bleung@chromium.org \
    --cc=ccross@android.com \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=enric.balletbo@collabora.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pasha.tatashin@soleen.com \
    --cc=robh+dt@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sashal@kernel.org \
    --cc=sergey.senozhatsky@gmail.com \
    --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: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git