[v2 PATCH] x86/mce: Fix timer interval adjustment after logging a MCE event

lirongqing posted 1 patch 3 weeks, 4 days ago
arch/x86/include/asm/mce.h         |  2 +-
arch/x86/kernel/cpu/mce/core.c     | 17 +++++++++++------
arch/x86/kernel/cpu/mce/intel.c    |  8 ++++++--
arch/x86/kernel/cpu/mce/internal.h |  2 +-
4 files changed, 19 insertions(+), 10 deletions(-)
[v2 PATCH] x86/mce: Fix timer interval adjustment after logging a MCE event
Posted by lirongqing 3 weeks, 4 days ago
From: Li RongQing <lirongqing@baidu.com>

Since commit 011d82611172 ("RAS: Add a Corrected Errors Collector"),
mce_timer_fn() has incorrectly determined whether to adjust the
timer interval. The issue arises because mce_notify_irq() now always
returns false when called from the timer path, since the polling code
never sets bit 0 of mce_need_notify. This prevents proper adjustment of
the timer interval based on whether MCE events were logged.

The mce_notify_irq() is called from two contexts:
1. Early notifier block - correctly sets mce_need_notify
2. Timer function - never sets mce_need_notify, making it a noop
   (though logged errors are still processed through mce_log()->
    x86_mce_decoder_chain -> early notifier).

Fix this by modifying machine_check_poll() to return a boolean indicating
whether any MCE was logged, and updating mc_poll_banks() and related
functions to propagate this return value. Then, mce_timer_fn() can use
this direct return value instead of relying on mce_notify_irq() for
timer interval decisions.

This ensures the timer interval is correctly reduced when MCE events are
logged and increased when no events occur.

Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
Diff with v1: rewrite commit message

 arch/x86/include/asm/mce.h         |  2 +-
 arch/x86/kernel/cpu/mce/core.c     | 17 +++++++++++------
 arch/x86/kernel/cpu/mce/intel.c    |  8 ++++++--
 arch/x86/kernel/cpu/mce/internal.h |  2 +-
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 2d98886..fb9eab4 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -303,7 +303,7 @@ enum mcp_flags {
 	MCP_QUEUE_LOG	= BIT(2),	/* only queue to genpool */
 };
 
-void machine_check_poll(enum mcp_flags flags, mce_banks_t *b);
+bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b);
 
 DECLARE_PER_CPU(struct mce, injectm);
 
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 3444002..8d42691 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -813,10 +813,11 @@ static void clear_bank(struct mce *m)
  * is already totally * confused. In this case it's likely it will
  * not fully execute the machine check handler either.
  */
-void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
+bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 {
 	struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
 	struct mce_hw_err err;
+	bool logged = false;
 	struct mce *m;
 	int i;
 
@@ -868,6 +869,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		else
 			mce_log(&err);
 
+		logged = true;
 clear_it:
 		clear_bank(m);
 	}
@@ -878,6 +880,8 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 	 */
 
 	sync_core();
+
+	return logged;
 }
 EXPORT_SYMBOL_GPL(machine_check_poll);
 
@@ -1776,12 +1780,12 @@ static void __start_timer(struct timer_list *t, unsigned long interval)
 	local_irq_restore(flags);
 }
 
-static void mc_poll_banks_default(void)
+static bool mc_poll_banks_default(void)
 {
-	machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
+	return machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
 }
 
-void (*mc_poll_banks)(void) = mc_poll_banks_default;
+bool (*mc_poll_banks)(void) = mc_poll_banks_default;
 
 static bool should_enable_timer(unsigned long iv)
 {
@@ -1792,19 +1796,20 @@ static void mce_timer_fn(struct timer_list *t)
 {
 	struct timer_list *cpu_t = this_cpu_ptr(&mce_timer);
 	unsigned long iv;
+	bool logged = false;
 
 	WARN_ON(cpu_t != t);
 
 	iv = __this_cpu_read(mce_next_interval);
 
 	if (mce_available(this_cpu_ptr(&cpu_info)))
-		mc_poll_banks();
+		logged = mc_poll_banks();
 
 	/*
 	 * Alert userspace if needed. If we logged an MCE, reduce the polling
 	 * interval, otherwise increase the polling interval.
 	 */
-	if (mce_notify_irq())
+	if (logged)
 		iv = max(iv / 2, (unsigned long) HZ/100);
 	else
 		iv = min(iv * 2, round_jiffies_relative(check_interval * HZ));
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index 4655223..a3d2730 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -395,11 +395,15 @@ void cmci_disable_bank(int bank)
 }
 
 /* Bank polling function when CMCI is disabled. */
-static void cmci_mc_poll_banks(void)
+static bool cmci_mc_poll_banks(void)
 {
+	bool logged;
+
 	spin_lock(&cmci_poll_lock);
-	machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
+	logged = machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
 	spin_unlock(&cmci_poll_lock);
+
+	return logged;
 }
 
 void intel_init_cmci(void)
diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index a31cf98..7bf2360 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -348,5 +348,5 @@ static __always_inline u32 mca_msr_reg(int bank, enum mca_msr reg)
 	return 0;
 }
 
-extern void (*mc_poll_banks)(void);
+extern bool (*mc_poll_banks)(void);
 #endif /* __X86_MCE_INTERNAL_H__ */
-- 
2.9.4
Re: [v2 PATCH] x86/mce: Fix timer interval adjustment after logging a MCE event
Posted by Borislav Petkov 3 weeks, 4 days ago
On Tue, Jan 13, 2026 at 02:05:06AM -0500, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> Since commit 011d82611172 ("RAS: Add a Corrected Errors Collector"),
> mce_timer_fn() has incorrectly determined whether to adjust the
> timer interval. The issue arises because mce_notify_irq() now always
> returns false when called from the timer path, since the polling code
> never sets bit 0 of mce_need_notify. This prevents proper adjustment of
> the timer interval based on whether MCE events were logged.
> 
> The mce_notify_irq() is called from two contexts:
> 1. Early notifier block - correctly sets mce_need_notify
> 2. Timer function - never sets mce_need_notify, making it a noop
>    (though logged errors are still processed through mce_log()->
>     x86_mce_decoder_chain -> early notifier).
> 
> Fix this by modifying machine_check_poll() to return a boolean indicating
> whether any MCE was logged, and updating mc_poll_banks() and related
> functions to propagate this return value. Then, mce_timer_fn() can use
> this direct return value instead of relying on mce_notify_irq() for
> timer interval decisions.
> 
> This ensures the timer interval is correctly reduced when MCE events are
> logged and increased when no events occur.
> 
> Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> Diff with v1: rewrite commit message
> 
>  arch/x86/include/asm/mce.h         |  2 +-
>  arch/x86/kernel/cpu/mce/core.c     | 17 +++++++++++------
>  arch/x86/kernel/cpu/mce/intel.c    |  8 ++++++--
>  arch/x86/kernel/cpu/mce/internal.h |  2 +-
>  4 files changed, 19 insertions(+), 10 deletions(-)

We're discussing the issue here:

https://lore.kernel.org/r/268e2f0512db435685af987a2ba6893c@baidu.com

Why are you sending another patch before we haven't agreed on whether there's
an issue in the first place?!

NAK!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette