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

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.

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.

A possible solution to the latter would be to have a "handled" or "printed"
flag within "struct mce" and print the MCE based on that in the default
handler. What do you think?
---
 arch/x86/kernel/cpu/mce/core.c | 90 ++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 38 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index d8fe5b048ee7..3b6e37c5252f 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -156,36 +156,6 @@ void mce_log(struct mce *m)
 }
 EXPORT_SYMBOL_GPL(mce_log);
 
-/*
- * We run the default notifier as long as we have no external
- * notifiers registered on the chain.
- */
-static atomic_t num_notifiers;
-
-static void mce_register_decode_chain_internal(struct notifier_block *nb)
-{
-	if (WARN_ON(nb->priority > MCE_PRIO_MCELOG && nb->priority < MCE_PRIO_EDAC))
-		return;
-
-	blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
-}
-
-void mce_register_decode_chain(struct notifier_block *nb)
-{
-	atomic_inc(&num_notifiers);
-
-	mce_register_decode_chain_internal(nb);
-}
-EXPORT_SYMBOL_GPL(mce_register_decode_chain);
-
-void mce_unregister_decode_chain(struct notifier_block *nb)
-{
-	atomic_dec(&num_notifiers);
-
-	blocking_notifier_chain_unregister(&x86_mce_decoder_chain, nb);
-}
-EXPORT_SYMBOL_GPL(mce_unregister_decode_chain);
-
 static inline u32 ctl_reg(int bank)
 {
 	return MSR_IA32_MCx_CTL(bank);
@@ -606,18 +576,19 @@ static struct notifier_block mce_uc_nb = {
 	.priority	= MCE_PRIO_UC,
 };
 
+/*
+ * We run the default notifier as long as we have no external
+ * notifiers registered on the chain.
+ */
+static atomic_t num_notifiers;
+
 static int mce_default_notifier(struct notifier_block *nb, unsigned long val,
 				void *data)
 {
 	struct mce *m = (struct mce *)data;
 
-	if (!m)
-		return NOTIFY_DONE;
-
-	if (atomic_read(&num_notifiers))
-		return NOTIFY_DONE;
-
-	__print_mce(m);
+	if (m)
+		__print_mce(m);
 
 	return NOTIFY_DONE;
 }
@@ -628,6 +599,49 @@ static struct notifier_block mce_default_nb = {
 	.priority	= MCE_PRIO_LOWEST,
 };
 
+static void update_default_notifier_registration(void)
+{
+	bool has_notifiers = !!atomic_read(&num_notifiers);
+
+retry:
+	if (has_notifiers)
+		blocking_notifier_chain_unregister(&x86_mce_decoder_chain,
+						   &mce_default_nb);
+	else
+		blocking_notifier_chain_cond_register(&x86_mce_decoder_chain,
+						      &mce_default_nb);
+
+	if (has_notifiers != !!atomic_read(&num_notifiers)) {
+		has_notifiers = !has_notifiers;
+		goto retry;
+	}
+}
+
+static void mce_register_decode_chain_internal(struct notifier_block *nb)
+{
+	if (WARN_ON(nb->priority > MCE_PRIO_MCELOG &&
+		    nb->priority < MCE_PRIO_EDAC))
+		return;
+
+	blocking_notifier_chain_register(&x86_mce_decoder_chain, nb);
+}
+
+void mce_register_decode_chain(struct notifier_block *nb)
+{
+	atomic_inc(&num_notifiers);
+	mce_register_decode_chain_internal(nb);
+	update_default_notifier_registration();
+}
+EXPORT_SYMBOL_GPL(mce_register_decode_chain);
+
+void mce_unregister_decode_chain(struct notifier_block *nb)
+{
+	atomic_dec(&num_notifiers);
+	update_default_notifier_registration();
+	blocking_notifier_chain_unregister(&x86_mce_decoder_chain, nb);
+}
+EXPORT_SYMBOL_GPL(mce_unregister_decode_chain);
+
 /*
  * Read ADDR and MISC registers.
  */
@@ -1972,7 +1986,7 @@ int __init mcheck_init(void)
 	mce_register_decode_chain_internal(&first_nb);
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		mce_register_decode_chain_internal(&mce_uc_nb);
-	mce_register_decode_chain_internal(&mce_default_nb);
+	update_default_notifier_registration();
 	mcheck_vendor_init_severity();
 
 	INIT_WORK(&mce_work, mce_gen_pool_process);
-- 
2.22.0.3.gb49bb57c8208.dirty


  parent reply	other threads:[~2020-01-03 15:08 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 ` Jan H. Schönherr [this message]
2020-01-03 19:46   ` [PATCH v2 6/6] x86/mce: Dynamically register default MCE handler Luck, Tony
2020-01-03 21:42   ` Luck, Tony
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=20200103150722.20313-7-jschoenh@amazon.de \
    --to=jschoenh@amazon.de \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --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).