Skip to contentSkip to conversation

x86: MCE: Re-implement MCE log ring buffer as per-CPU ring buffer

PATCH In review Huang Ying 2 participants 3 messages Aug 20, 2009 – Aug 21, 2009 x86

lore
Share & tools
lore

Huang Ying

author
#0 lore raw

PATCH 1/1 x86: MCE: Re-implement MCE log ring buffer as per-CPU ring buffer

Hi,

We encountered several old mce ring buffer related problems recently. In addition to the known livelock we seem to have a strange data corruption problem.

But, it will take some time for unified tracing ring buffer to go lock-less, and use that in MCE code. To solve the current problems, we suggest to use this implementation as a temporary solution and go unified tracing buffer later.

Best Regards,
Huang Ying -------------------------------------------------------------> Re-implement MCE log ring buffer as per-CPU ring buffer for better scalability.

AK: the main reason to add the patch is less that performance (normally there shouldn't be that many errors, so it doesn't matter too much), but that it scales to a larger number of CPUs. The old mce log ring had a fixed sized buffer that was hard to extend and didn't scale on larger systems, which could cause overflows and other issues. Also this fixes a long standing known livelock in the old code where a very high continuous error rate could livelock the reader.

Basic design is as follow:

- One ring buffer for each CPU

+ MCEs are added to corresponding local per-CPU buffer, instead of one big global buffer. Contention/unfairness between CPUs is eleminated.

+ MCE records are read out and removed from per-CPU buffers by mutex protected global reader function. Because there are no many readers in system to contend in most cases.

- Per-CPU ring buffer data structure

+ An array is used to hold MCE records. integer "head" indicates next writing position and integer "tail" indicates next reading position.

+ To distinguish buffer empty and full, head and tail wrap to 0 at MCE_LOG_LIMIT instead of MCE_LOG_LEN. Then the real next writing position is head % MCE_LOG_LEN, and real next reading position is tail % MCE_LOG_LEN. If buffer is empty, head == tail, if buffer is full, head % MCE_LOG_LEN == tail % MCE_LOG_LEN and head != tail.

- Lock-less for writer side

+ MCE log writer may come from NMI, so the writer side must be lock-less. For per-CPU buffer of one CPU, writers may come from process, IRQ or NMI context, so "head" is increased with cmpxchg_local() to allocate buffer space.

+ Reader side is protected with a mutex to guarantee only one reader is active in the whole system.

Performance test show that the throughput of per-CPU mcelog buffer can reach 430k records/s compared with 5.3k records/s for original implementation on a 2-core 2.1GHz Core2 machine.

ChangeLog:

v6:

- Rebased on latest x86-tip.git/x86/mce + x86/tip

v5:

- Rebased on latest x86-tip.git/mce4.

- Fix parsing mce_log data structure in dumping kernel.

v4:

- Rebased on x86-tip.git x86/mce3 branch

- Fix a synchronization issue about mce.finished

- Fix comment style

v3:

- Use DEFINE_PER_CPU to allocate per_cpu mcelog buffer.

- Use cond_resched() to prevent possible system not reponsing issue for large user buffer.

v2:

- Use alloc_percpu() to allocate per_cpu mcelog buffer.

- Use ndelay to implement witer timeout.

---
 arch/x86/include/asm/mce.h       |   18 ++-
 arch/x86/kernel/cpu/mcheck/mce.c |  293 +++++++++++++++++++++++---------------
 2 files changed, 190 insertions(+), 121 deletions(-)
arch/x86/include/asm/mce.h +13 -5
index b608a64..0ea3d41 100644
@@ -76,20 +76,28 @@ struct mce {
  * is set.
  */
 
-#define MCE_LOG_LEN 32
+#define MCE_LOG_LEN		32
+#define MCE_LOG_LIMIT		(MCE_LOG_LEN * 2 - 1)
+
+static inline int mce_log_index(int n)
+{
+	return n >= MCE_LOG_LEN ? n - MCE_LOG_LEN : n;
+}
+
+struct mce_log_cpu;
 
 struct mce_log {
-	char signature[12]; /* "MACHINECHECK" */
+	char signature[12]; /* "MACHINECHEC2" */
 	unsigned len;	    /* = MCE_LOG_LEN */
-	unsigned next;
 	unsigned flags;
 	unsigned recordlen;	/* length of struct mce */
-	struct mce entry[MCE_LOG_LEN];
+	unsigned nr_mcelog_cpus;
+	struct mce_log_cpu **mcelog_cpus;
 };
 
 #define MCE_OVERFLOW 0		/* bit 0 in flags means overflow */
 
-#define MCE_LOG_SIGNATURE	"MACHINECHECK"
+#define MCE_LOG_SIGNATURE	"MACHINECHEC2"
 
 #define MCE_GET_RECORD_LEN   _IOR('M', 1, int)
 #define MCE_GET_LOG_LEN      _IOR('M', 2, int)
arch/x86/kernel/cpu/mcheck/mce.c +177 -116

367 lines not on this page · raw message

#1 lore raw
Quoted 3 lines · Huang Ying

Re-implement MCE log ring buffer as per-CPU ring buffer for better scalability.

(snip)

Quoted 5 lines

ChangeLog:

v6:

- Rebased on latest x86-tip.git/x86/mce + x86/tip

Does this mean your patch is based on a local branch you merged? Why not on tip/x86/mce alone as usual?
What branch are you requesting this patch to be applied to?

Thanks,
H.Seto

author
#2 lore raw
Quoted 7 lines · Hidetoshi Seto
Quoted 3 lines · Huang Ying

Re-implement MCE log ring buffer as per-CPU ring buffer for better scalability.

(snip)

Quoted 2 lines

ChangeLog:

Quoted 7 lines
Quoted 3 lines

v6:

- Rebased on latest x86-tip.git/x86/mce + x86/tip

Does this mean your patch is based on a local branch you merged? Why not on tip/x86/mce alone as usual?
What branch are you requesting this patch to be applied to?

Re-checked again. The patch can be applied to x86/mce, with only one not important "fuzzy 1" warning.

Best Regards,
Huang Ying