Linux EDAC development
 help / color / mirror / Atom feed
* [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
@ 2026-09-03  4:13 Aaron Tomlin
  2026-09-03  4:13 ` [PATCH v4 1/2] x86/mce: Do not reinitialise mce_timer structure on CPU restart Aaron Tomlin
  2026-09-03  4:13 ` [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required Aaron Tomlin
  0 siblings, 2 replies; 6+ messages in thread
From: Aaron Tomlin @ 2026-09-03  4:13 UTC (permalink / raw)
  To: tony.luck, bp, tglx, mingo, dave.hansen
  Cc: x86, hpa, frederic, marco.crivellari, neelx, sean, chjohnst,
	mproche, nick.lange, linux-edac, linux-kernel

This series addresses two distinct issues within the x86 Machine Check
Architecture (MCA) timer subsystem: a race condition during runtime CPU
reconfiguration that can corrupt the kernel timer wheel, and redundant
periodic software polling of banks that never log corrected errors.

Patch 1 fixes a concurrency race between sysfs configuration updates
(mce_restart()) and asynchronous CMCI interrupts. When mce_restart() runs,
a concurrent CMCI interrupt can arm mce_timer on a remote CPU before the
restart IPI arrives. By removing the redundant timer_setup() call from
__mcheck_cpu_init_timer(), Patch 1 ensures that mce_timer descriptors are
not re-initialised while actively linked in the timer wheel, avoiding
potential linked-list corruption and kernel crashes.

Patch 2 implements Tony Luck's suggested approach by recognising that banks
without CMCI support on modern Intel platforms (such as the PCU bank) never
report corrected or UCNA errors (per Intel SDM Vol 3B 18.5). It clears
these non-CMCI banks from mce_poll_banks and ensures mce_timer is never
armed when mce_poll_banks is empty. Additionally, it integrates a
housekeeping check (HK_TYPE_TIMER) so that on legacy platforms or
polling-only configurations where mce_poll_banks is non-empty, routine
polling is restricted to housekeeping CPUs, sparing isolated nohz_full
cores from timer interrupts. This eliminates polling timer jitter across
all CPUs in steady state on modern hardware while preserving full polling
capabilities and isolation guarantees.

Thank you.

Changes since v3:

 - Removed redundant code since field poll_only of struct storm_bank is no
   longer set

 - Link to v3: https://lore.kernel.org/lkml/20260903013933.172063-1-atomlin@atomlin.com/

Changes since v2:

 - Bounded bitmap_empty() in should_enable_timer() to
   this_cpu_read(mce_num_banks) to prevent initialised upper bits from
   keeping the timer active (Tony Luck)

 - Clarified that on polling fallback systems, restricting mce_timer to
   housekeeping CPUs leaves core-private banks on isolated cores exempt
   from polling, while preserving shared platform telemetry
   (e.g., Memory Controller ECC)

 - Link to v2: https://lore.kernel.org/lkml/20260902020234.149814-1-atomlin@atomlin.com/

Changes since v1:

 - Fixed a pre-existing race condition in mce_restart() by removing the
   redundant timer_setup() call in __mcheck_cpu_init_timer(), preventing
   active timer wheel linked-list corruption

 - Non-CMCI banks are cleared from mce_poll_banks in cmci_claim_bank(),
   and should_enable_timer() verifies bitmap_empty(mce_poll_banks) before
   checking HK_TYPE_TIMER

 - Link to v1: https://lore.kernel.org/lkml/20260901151138.132950-1-atomlin@atomlin.com/

Aaron Tomlin (2):
  x86/mce: Do not reinitialise mce_timer structure on CPU restart
  x86/mce: Avoid arming periodic polling timer when not required

 arch/x86/kernel/cpu/mce/core.c      |  8 +++++++-
 arch/x86/kernel/cpu/mce/intel.c     | 11 ++++++-----
 arch/x86/kernel/cpu/mce/internal.h  |  2 --
 arch/x86/kernel/cpu/mce/threshold.c |  4 ----
 4 files changed, 13 insertions(+), 12 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/2] x86/mce: Do not reinitialise mce_timer structure on CPU restart
  2026-09-03  4:13 [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling Aaron Tomlin
@ 2026-09-03  4:13 ` Aaron Tomlin
  2026-09-03  4:13 ` [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required Aaron Tomlin
  1 sibling, 0 replies; 6+ messages in thread
From: Aaron Tomlin @ 2026-09-03  4:13 UTC (permalink / raw)
  To: tony.luck, bp, tglx, mingo, dave.hansen
  Cc: x86, hpa, frederic, marco.crivellari, neelx, sean, chjohnst,
	mproche, nick.lange, linux-edac, linux-kernel

During early CPU initialisation, mcheck_cpu_init() invokes
__mcheck_cpu_setup_timer(), which initialises the per-CPU mce_timer
descriptor via timer_setup(t, mce_timer_fn, TIMER_PINNED).

However, __mcheck_cpu_init_timer() is also called at runtime during
sysfs reconfiguration (via mce_cpu_restart() and mce_enable_ce()),
redundantly re-initialising mce_timer with timer_setup().

This introduces a race condition against asynchronous CMCI interrupts.
When mce_restart() runs, it invokes mce_timer_delete_all() before
broadcasting an IPI to execute mce_cpu_restart() on all CPUs. If a
hardware CMCI interrupt fires on another CPU before the IPI is serviced,
the CMCI handler triggers storm tracking and arms mce_timer via
mce_timer_kick(true), enqueuing it in the active timer wheel.

When mce_cpu_restart() runs subsequently in IPI context,
__mcheck_cpu_init_timer() calls timer_setup(), which invokes
__init_timer() on the already-queued timer. This clears the timer's
internal list pointers (entry.next and entry.pprev) whilst it remains
linked in the active timer wheel bucket, causing linked list corruption,
softirq lockups, or kernel panics during timer expiration.

Fix this by removing the redundant timer_setup() call from
__mcheck_cpu_init_timer(). The timer is already initialised once during
CPU setup; runtime restart paths only need to arm it via
mce_start_timer().

Fixes: 26c3c283c5b0 ("x86: mce: Split timer init")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 arch/x86/kernel/cpu/mce/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index ab469605fc89..765e8103b0d2 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2091,7 +2091,6 @@ static void __mcheck_cpu_init_timer(void)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
-	timer_setup(t, mce_timer_fn, TIMER_PINNED);
 	mce_start_timer(t);
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required
  2026-09-03  4:13 [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling Aaron Tomlin
  2026-09-03  4:13 ` [PATCH v4 1/2] x86/mce: Do not reinitialise mce_timer structure on CPU restart Aaron Tomlin
@ 2026-09-03  4:13 ` Aaron Tomlin
  2026-09-03  8:49   ` Marco Crivellari
  1 sibling, 1 reply; 6+ messages in thread
From: Aaron Tomlin @ 2026-09-03  4:13 UTC (permalink / raw)
  To: tony.luck, bp, tglx, mingo, dave.hansen
  Cc: x86, hpa, frederic, marco.crivellari, neelx, sean, chjohnst,
	mproche, nick.lange, linux-edac, linux-kernel

On x86 platforms, the Machine Check Architecture (MCA) subsystem arms a
per-CPU, pinned standard timer (mce_timer) to periodically poll hardware
banks for "silent" corrected machine check errors. Because mce_timer is
pinned to the local CPU via TIMER_PINNED, the timer core cannot migrate
its expiration to a housekeeping CPU, causing periodic latency jitter on
isolated cores.

On Intel systems supporting Corrected Machine Check Interrupt (CMCI),
hardware generates an interrupt for banks with CMCI enabled, clearing
their respective bits in mce_poll_banks. However, for banks where
MCI_CTL2_CMCI_EN does not stick (such as the Power Control Unit bank),
Linux historically assumed software polling was required and left the
bank flagged in mce_poll_banks.

Per the Intel SDM (Vol 3B, Section 18.5 "Corrected Machine Check Error
Interrupt"), if bit 30 of IA32_MCi_CTL2 is zero, no CMCI is available
for that bank and no corrected or Uncorrected No Action Required (UCNA)
errors will be reported on that bank. Therefore, polling such banks is
redundant and wasteful.

Clear non-CMCI banks from mce_poll_banks in cmci_claim_bank() on
CMCI-capable CPUs, and amend should_enable_timer() to:
    1.  Check bitmap_empty(mce_poll_banks) so that mce_timer is never
        armed when no banks on that CPU require software polling.

    2.  Pin the MCE polling timer to a HK_TYPE_TIMER housekeeping CPU,
        sparing isolated nohz_full cores from periodic wakeups on
        systems requiring mce_poll_banks polling.

On systems requiring software polling such legacy platforms lacking CMCI
or when booted with mce=no_cmci, the mce_timer is restricted to
housekeeping CPUs. While this intentionally leaves core-private banks
(e.g., L1/L2 caches) on isolated cores exempt from periodic polling to
guarantee zero timer jitter, housekeeping CPUs continue to harvest
telemetry from their own banks as well as shared package-level
resources. Synchronous (#MC) exceptions on isolated cores remain
entirely unaffected.

Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 arch/x86/kernel/cpu/mce/core.c      |  7 +++++++
 arch/x86/kernel/cpu/mce/intel.c     | 11 ++++++-----
 arch/x86/kernel/cpu/mce/internal.h  |  2 --
 arch/x86/kernel/cpu/mce/threshold.c |  4 ----
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 765e8103b0d2..b01ad1a9b455 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -25,6 +25,7 @@
 #include <linux/delay.h>
 #include <linux/ctype.h>
 #include <linux/sched.h>
+#include <linux/sched/isolation.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/slab.h>
@@ -1759,6 +1760,12 @@ void (*mc_poll_banks)(void) = mc_poll_banks_default;
 
 static bool should_enable_timer(unsigned long iv)
 {
+	if (bitmap_empty(this_cpu_ptr(mce_poll_banks), this_cpu_read(mce_num_banks)))
+		return false;
+
+	if (!housekeeping_cpu(smp_processor_id(), HK_TYPE_TIMER))
+		return false;
+
 	return !mca_cfg.ignore_ce && iv;
 }
 
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index 4655223ba560..cc7d0dfcf9c7 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -229,16 +229,17 @@ static u64 cmci_pick_threshold(u64 val, int *bios_zero_thresh)
  */
 static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_wrong_thresh)
 {
-	struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
-
 	val |= MCI_CTL2_CMCI_EN;
 	wrmsrq(MSR_IA32_MCx_CTL2(bank), val);
 	rdmsrq(MSR_IA32_MCx_CTL2(bank), val);
 
-	/* If the enable bit did not stick, this bank should be polled. */
+	/*
+	 * If the enable bit did not stick, this bank does not support CMCI
+	 * and no corrected or UCNA errors will be reported on this bank
+	 * (SDM Vol 3B 18.5). No polling is needed.
+	 */
 	if (!(val & MCI_CTL2_CMCI_EN)) {
-		WARN_ON(!test_bit(bank, this_cpu_ptr(mce_poll_banks)));
-		storm->banks[bank].poll_only = true;
+		__clear_bit(bank, this_cpu_ptr(mce_poll_banks));
 		return;
 	}
 
diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index a31cf984619c..32ae574d2ad5 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -84,13 +84,11 @@ static inline u32  mce_get_apei_thr_limit(void) { return 0; }
  *
  * timestamp:		Last time (in jiffies) that the bank was polled.
  * in_storm_mode:	Is this bank in storm mode?
- * poll_only:		Bank does not support CMCI, skip storm tracking.
  */
 struct storm_bank {
 	u64 history;
 	u64 timestamp;
 	bool in_storm_mode;
-	bool poll_only;
 };
 
 #define NUM_HISTORY_BITS (sizeof(u64) * BITS_PER_BYTE)
diff --git a/arch/x86/kernel/cpu/mce/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
index 6c370d5af5bd..2a82c9f6b5e4 100644
--- a/arch/x86/kernel/cpu/mce/threshold.c
+++ b/arch/x86/kernel/cpu/mce/threshold.c
@@ -118,10 +118,6 @@ void mce_track_storm(struct mce *mce)
 	unsigned int shift = 1;
 	u64 history = 0;
 
-	/* No tracking needed for banks that do not support CMCI */
-	if (storm->banks[mce->bank].poll_only)
-		return;
-
 	/*
 	 * When a bank is in storm mode it is polled once per second and
 	 * the history mask will record about the last minute of poll results.
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required
  2026-09-03  4:13 ` [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required Aaron Tomlin
@ 2026-09-03  8:49   ` Marco Crivellari
  2026-09-03 19:18     ` Aaron Tomlin
  0 siblings, 1 reply; 6+ messages in thread
From: Marco Crivellari @ 2026-09-03  8:49 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: tony.luck, bp, tglx, mingo, dave.hansen, x86, hpa, frederic,
	neelx, sean, chjohnst, mproche, nick.lange, linux-edac,
	linux-kernel

Hi Aaron,

On Thu, Sep 3, 2026 at 6:13 AM Aaron Tomlin <atomlin@atomlin.com> wrote:
>
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -229,16 +229,17 @@ static u64 cmci_pick_threshold(u64 val, int *bios_zero_thresh)
>   */
>  static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_wrong_thresh)
>  {
> -       struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
> -
>         val |= MCI_CTL2_CMCI_EN;
>         wrmsrq(MSR_IA32_MCx_CTL2(bank), val);
>         rdmsrq(MSR_IA32_MCx_CTL2(bank), val);
>
> -       /* If the enable bit did not stick, this bank should be polled. */
> +       /*
> +        * If the enable bit did not stick, this bank does not support CMCI
> +        * and no corrected or UCNA errors will be reported on this bank
> +        * (SDM Vol 3B 18.5). No polling is needed.
> +        */
>         if (!(val & MCI_CTL2_CMCI_EN)) {
> -               WARN_ON(!test_bit(bank, this_cpu_ptr(mce_poll_banks)));
> -               storm->banks[bank].poll_only = true;
> +               __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
>                 return;
>         }

Not sure Sashiko's review it makes sense but to me it could make sense [1].
In short:

> cmci_storm_begin()
>    __set_bit(bank, this_cpu_ptr(mce_poll_banks));
> cmci_storm_end()
>    __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
>
> Since __clear_bit() and __set_bit() are non-atomic read-modify-write operations
> and softirqs run with hardirqs enabled, will the hardirq's modification be
> overwritten and lost when the softirq resumes and writes back its value?

Sashiko's suggestion is to use the atomic versions, "clear_bit()" and
"set_bit()".

Thanks!

- [1] https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com

-- 

Marco Crivellari

SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required
  2026-09-03  8:49   ` Marco Crivellari
@ 2026-09-03 19:18     ` Aaron Tomlin
  2026-09-03 19:43       ` Luck, Tony
  0 siblings, 1 reply; 6+ messages in thread
From: Aaron Tomlin @ 2026-09-03 19:18 UTC (permalink / raw)
  To: Marco Crivellari
  Cc: tony.luck, bp, tglx, mingo, dave.hansen, x86, hpa, frederic,
	neelx, sean, chjohnst, mproche, nick.lange, linux-edac,
	linux-kernel

On Thu, Sep 03, 2026 at 10:49:34AM +0200, Marco Crivellari wrote:
> Hi Aaron,
> 
> On Thu, Sep 3, 2026 at 6:13 AM Aaron Tomlin <atomlin@atomlin.com> wrote:
> >
> > +++ b/arch/x86/kernel/cpu/mce/intel.c
> > @@ -229,16 +229,17 @@ static u64 cmci_pick_threshold(u64 val, int *bios_zero_thresh)
> >   */
> >  static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_wrong_thresh)
> >  {
> > -       struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
> > -
> >         val |= MCI_CTL2_CMCI_EN;
> >         wrmsrq(MSR_IA32_MCx_CTL2(bank), val);
> >         rdmsrq(MSR_IA32_MCx_CTL2(bank), val);
> >
> > -       /* If the enable bit did not stick, this bank should be polled. */
> > +       /*
> > +        * If the enable bit did not stick, this bank does not support CMCI
> > +        * and no corrected or UCNA errors will be reported on this bank
> > +        * (SDM Vol 3B 18.5). No polling is needed.
> > +        */
> >         if (!(val & MCI_CTL2_CMCI_EN)) {
> > -               WARN_ON(!test_bit(bank, this_cpu_ptr(mce_poll_banks)));
> > -               storm->banks[bank].poll_only = true;
> > +               __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
> >                 return;
> >         }
> 
> Not sure Sashiko's review it makes sense but to me it could make sense [1].
> In short:
> 
> > cmci_storm_begin()
> >    __set_bit(bank, this_cpu_ptr(mce_poll_banks));
> > cmci_storm_end()
> >    __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
> >
> > Since __clear_bit() and __set_bit() are non-atomic read-modify-write operations
> > and softirqs run with hardirqs enabled, will the hardirq's modification be
> > overwritten and lost when the softirq resumes and writes back its value?
> 
> Sashiko's suggestion is to use the atomic versions, "clear_bit()" and
> "set_bit()".
> 
> Thanks!
> 
> - [1] https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com
> 
> -- 
> 
> Marco Crivellari
> 
> SUSE Labs

Hi Marco,

Thank you for your review!

Yes, the per-CPU variable mce_poll_banks can indeed be modified across
different execution contexts on the same CPU.

Tony, I will address what Sashiko [1] reported in the next iteration.

[1]: https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com

Kind regards,
-- 
Aaron Tomlin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required
  2026-09-03 19:18     ` Aaron Tomlin
@ 2026-09-03 19:43       ` Luck, Tony
  0 siblings, 0 replies; 6+ messages in thread
From: Luck, Tony @ 2026-09-03 19:43 UTC (permalink / raw)
  To: Aaron Tomlin, Marco Crivellari
  Cc: bp@alien8.de, tglx@kernel.org, mingo@redhat.com,
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
	frederic@kernel.org, neelx@suse.com, sean@ashe.io,
	chjohnst@gmail.com, mproche@gmail.com, nick.lange@gmail.com,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org

> Yes, the per-CPU variable mce_poll_banks can indeed be modified across
> different execution contexts on the same CPU.
>
> Tony, I will address what Sashiko [1] reported in the next iteration.
>
> [1]: https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com

Hi Aaron,

OK. It's a separate issue, so put the change in it's own patch.

Patches 1 & 2 now look to be in good shape.

-Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-03 19:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  4:13 [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling Aaron Tomlin
2026-09-03  4:13 ` [PATCH v4 1/2] x86/mce: Do not reinitialise mce_timer structure on CPU restart Aaron Tomlin
2026-09-03  4:13 ` [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required Aaron Tomlin
2026-09-03  8:49   ` Marco Crivellari
2026-09-03 19:18     ` Aaron Tomlin
2026-09-03 19:43       ` Luck, Tony

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox