linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: "Jan H. Schönherr" <jschoenh@amazon.de>
Cc: Borislav Petkov <bp@alien8.de>,
	Yazen Ghannam <yazen.ghannam@amd.com>,
	linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org
Subject: Re: [PATCH v2 6/6] x86/mce: Dynamically register default MCE handler
Date: Fri, 3 Jan 2020 13:42:29 -0800	[thread overview]
Message-ID: <20200103214229.GA10677@agluck-desk2.amr.corp.intel.com> (raw)
In-Reply-To: <20200103150722.20313-7-jschoenh@amazon.de>

On Fri, Jan 03, 2020 at 04:07:22PM +0100, Jan H. Schönherr wrote:
> The default MCE handler takes action, when no external MCE handler is
> registered. Instead of checking for external handlers within the default
> MCE handler, only register the default MCE handler when there are no
> external handlers in the first place.
> 
> Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
> ---
> New in v2. This is something that became possible due to patch 4.
> I'm not entirely happy with it.
> 
> One the one hand, I'm wondering whether there's a nicer way to handle
> (de-)registration races.
> 

Instead of unregistering/registering the default notifier depending
on whether there are other notifiers, couldn't you just make the
default notifier check to see if it should print. E.g.

static int mce_default_notifier(struct notifier_block *nb, unsigned long val,
			void *data)
{
	struct mce *m = (struct mce *)data;

	if (m && !atomic_read(&num_notifiers))
		__print_mce(m);

	return NOTIFY_DONE;
}

> On the other hand, I'm starting to question the whole logic to "only print
> the MCE if nothing else in the kernel has a handler registered". Is that
> really how it should be? For example, there are handlers that filter for a
> specific subset of MCEs. If one of those is registered, we're losing all
> information for MCEs that don't match.

Maybe put control of this into the hands of the notifiers ... if
a notifier thinks that it does something useful with the log
that makes the console log redundant, then it could call a function
to bump the count to suppress the __print_mce(). Simple filter functions
on the chain wouldn't do that.

If we go this path the variable should be named something like
"suppress_console_mce" rather than num_notifiers.

Might also be useful to have some command line option or debugfs knob
to force printing for those cases where bad stuff is happening and we
don't see what was logged before a crash drops all the bits on the floor.

-Tony

  parent reply	other threads:[~2020-01-03 21:42 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-03 15:07 [PATCH v2 0/6] x86/mce: Various fixes and cleanups for MCE handling Jan H. Schönherr
2020-01-03 15:07 ` [PATCH v2 1/6] x86/mce: Take action on UCNA/Deferred errors again Jan H. Schönherr
2020-01-10  9:50   ` Borislav Petkov
2020-01-10 18:45     ` Luck, Tony
2020-01-11 13:06       ` Borislav Petkov
2020-01-11 13:17       ` Borislav Petkov
2020-01-03 15:07 ` [PATCH v2 2/6] x86/mce: Make mce=nobootlog work again Jan H. Schönherr
2020-01-03 15:07 ` [PATCH v2 3/6] x86/mce: Fix use of uninitialized MCE message string Jan H. Schönherr
2020-01-03 15:07 ` [PATCH v2 4/6] x86/mce: Allow a variable number of internal MCE decode notifiers Jan H. Schönherr
2020-01-03 15:07 ` [PATCH v2 5/6] x86/mce: Do not take action on SRAO/Deferred errors on AMD for now Jan H. Schönherr
2020-01-03 15:07 ` [PATCH v2 6/6] x86/mce: Dynamically register default MCE handler Jan H. Schönherr
2020-01-03 19:46   ` Luck, Tony
2020-01-03 21:42   ` Luck, Tony [this message]
2020-01-03 22:03   ` Borislav Petkov
2020-01-08  4:24     ` Ghannam, Yazen
2020-01-08 10:03       ` Borislav Petkov
2020-01-09 20:39         ` Ghannam, Yazen
2020-01-09 21:54           ` Luck, Tony
2020-01-09 22:30             ` Jan H. Schönherr
2020-01-04 11:49   ` kbuild test robot

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=20200103214229.GA10677@agluck-desk2.amr.corp.intel.com \
    --to=tony.luck@intel.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jschoenh@amazon.de \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yazen.ghannam@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).