x86: mce: Xeon75xx specific interface to get corrected memory error information

x86: mce: Xeon75xx specific interface to get corrected memory error information

Xeon 75xx doesn't log physical addresses on corrected machine check events in the standard architectural MSRs. Instead the address has to be retrieved in a model specific way. This makes it impossible to do predictive failure analysis.

Implement cpu model specific code to do this in mce-xeon75xx.c using a new hook that is called from the generic poll code. The code retrieves
the physical address/DIMM of the last corrected error from the platform and makes the address look like a standard architectural MCA address for further processing.

In addition the DIMM information is retrieved and put into two new aux0/aux1 fields in struct mce. These fields are specific to a given CPU. These fields can then be decoded by mcelog into specific DIMM information. The latest mcelog version has support for this.

Longer term this will be likely in a different output format, but short term that seemed like the least intrusive solution. Older mcelog can deal with an extended record.

There's no code to print this information on a panic because this only works for corrected errors, and corrected errors do not usually result in panics.

The act of retrieving the DIMM/PA information can take some time, so this code has a rate limit to avoid taking too much CPU time on a error flood.

The whole thing can be loaded as a module and has suitable PCI-IDs so that it can be auto-loaded by a distribution. The code also checks explicitely for the expected CPU model number to make sure this code doesn't run anywhere else.

---
 arch/x86/Kconfig                          |    8 
 arch/x86/include/asm/mce.h                |    2 
 arch/x86/kernel/cpu/mcheck/Makefile       |    1 
 arch/x86/kernel/cpu/mcheck/mce-internal.h |    1 
 arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c |  427 ++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c          |   12 
 arch/x86/kernel/e820.c                    |    3 
 7 files changed, 453 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce.c
===================================================================
@@ -88,6 +88,9 @@ static DECLARE_WAIT_QUEUE_HEAD(mce_wait)
 static DEFINE_PER_CPU(struct mce, mces_seen);
 static int			cpu_missing;
 
+void 				(*cpu_specific_poll)(struct mce *);
+EXPORT_SYMBOL_GPL(cpu_specific_poll);
+
 /*
  * CPU/chipset specific EDAC code can register a notifier call here to print
  * MCE errors in a human-readable form.

@@ -364,6 +367,11 @@ static void mce_wrmsrl(u32 msr, u64 v)
 	wrmsrl(msr, v);
 }
 
+static int under_injection(void)
+{
+	return __get_cpu_var(injectm).finished;
+}
+
 /*
  * Simple lockless ring to communicate PFNs from the exception handler with the
  * process context work function. This is vastly simplified because there's

@@ -575,6 +583,10 @@ void machine_check_poll(enum mcp_flags f
 
 		if (!(flags & MCP_TIMESTAMP))
 			m.tsc = 0;
+
+		if (cpu_specific_poll && !under_injection() && !mce_dont_log_ce)
+			cpu_specific_poll(&m);
+
 		/*
 		 * Don't get the IP here because it's unlikely to
 		 * have anything to do with the actual error location.

Index: linux/arch/x86/kernel/cpu/mcheck/mce-internal.h
===================================================================
@@ -28,3 +28,4 @@ extern int mce_ser;
 
 extern struct mce_bank *mce_banks;
 
+extern void (*cpu_specific_poll)(struct mce *);

Index: linux/arch/x86/include/asm/mce.h
===================================================================
@@ -67,6 +67,8 @@ struct mce {
 	__u32 socketid;	/* CPU socket ID */
 	__u32 apicid;	/* CPU initial apic ID */
 	__u64 mcgcap;	/* MCGCAP MSR: machine check capabilities of CPU */
+	__u64 aux0;	/* model specific */
+	__u64 aux1;	/* model specific */
 };
 
 /*

Index: linux/arch/x86/kernel/cpu/mcheck/Makefile
===================================================================
@@ -2,6 +2,7 @@ obj-y				=  mce.o mce-severity.o
 
 obj-$(CONFIG_X86_ANCIENT_MCE)	+= winchip.o p5.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_MCE_XEON75XX)	+= mce-xeon75xx.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
 obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o

Index: linux/arch/x86/kernel/e820.c
===================================================================
@@ -79,7 +79,7 @@ EXPORT_SYMBOL_GPL(e820_any_mapped);
  * Note: this function only works correct if the e820 table is sorted and
  * not-overlapping, which is the case
  */
-int __init e820_all_mapped(u64 start, u64 end, unsigned type)
+int e820_all_mapped(u64 start, u64 end, unsigned type)
 {
 	int i;
 

@@ -106,6 +106,7 @@ int __init e820_all_mapped(u64 start, u6
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(e820_all_mapped);
 
 /*
  * Add a memory region to the kernel e820 map.

Index: linux/arch/x86/Kconfig
===================================================================
@@ -831,6 +831,14 @@ config X86_MCE_INTEL
 	   Additional support for intel specific MCE features such as
 	   the thermal monitor.
 
+config X86_MCE_XEON75XX
+	tristate "Intel Xeon 7500 series corrected memory error driver"
+	depends on X86_MCE_INTEL
+	---help---
+	   Add support for a Intel Xeon 7500 series specific memory error driver.
+	   This allows to report the DIMM and physical address on a corrected
+	   memory error machine check event.
+
 config X86_MCE_AMD
 	def_bool y
 	prompt "AMD MCE features"

Commit-ID:  c773f70fd6b53ee646727f871833e53649907264
Gitweb:     http://git.kernel.org/tip/c773f70fd6b53ee646727f871833e53649907264
Author:     Andi Kleen <[email protected]>
AuthorDate: Thu, 21 Jan 2010 23:17:12 +0100
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Thu, 21 Jan 2010 17:38:20 -0800

x86, mce: Xeon75xx specific interface to get corrected memory error information

Xeon 75xx doesn't log physical addresses on corrected machine check events in the standard architectural MSRs. Instead the address has to be retrieved in a model specific way. This makes it impossible to do predictive failure analysis.

Implement cpu model specific code to do this in mce-xeon75xx.c using a new hook that is called from the generic poll code. The code retrieves the physical address/DIMM of the last corrected error from the platform and makes the address look like a standard architectural MCA address for further processing.

In addition the DIMM information is retrieved and put into two new aux0/aux1 fields in struct mce. These fields are specific to a given CPU. These fields can then be decoded by mcelog into specific DIMM information. The latest mcelog version has support for this.

Longer term this will be likely in a different output format, but short term that seemed like the least intrusive solution. Older mcelog can deal with an extended record.

There's no code to print this information on a panic because this only works for corrected errors, and corrected errors do not usually result in panics.

The act of retrieving the DIMM/PA information can take some time, so this code has a rate limit to avoid taking too much CPU time on a error flood.

The whole thing can be loaded as a module and has suitable PCI-IDs so that it can be auto-loaded by a distribution. The code also checks explicitely for the expected CPU model number to make sure this code doesn't run anywhere else.

LKML-Reference: <[email protected]>

---
 arch/x86/Kconfig                          |    8 +
 arch/x86/include/asm/mce.h                |    2 +
 arch/x86/kernel/cpu/mcheck/Makefile       |    1 +
 arch/x86/kernel/cpu/mcheck/mce-internal.h |    1 +
 arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c |  427 +++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/mcheck/mce.c          |   12 +
 arch/x86/kernel/e820.c                    |    3 +-
 7 files changed, 453 insertions(+), 1 deletions(-)

index cbcbfde..f62db24 100644
@@ -832,6 +832,14 @@ config X86_MCE_INTEL
 	   Additional support for intel specific MCE features such as
 	   the thermal monitor.
 
+config X86_MCE_XEON75XX
+	tristate "Intel Xeon 7500 series corrected memory error driver"
+	depends on X86_MCE_INTEL
+	---help---
+	   Add support for a Intel Xeon 7500 series specific memory error driver.
+	   This allows to report the DIMM and physical address on a corrected
+	   memory error machine check event.
+
 config X86_MCE_AMD
 	def_bool y
 	prompt "AMD MCE features"

index 6c3fdd6..d2cf043 100644
@@ -67,6 +67,8 @@ struct mce {
 	__u32 socketid;	/* CPU socket ID */
 	__u32 apicid;	/* CPU initial apic ID */
 	__u64 mcgcap;	/* MCGCAP MSR: machine check capabilities of CPU */
+	__u64 aux0;	/* model specific */
+	__u64 aux1;	/* model specific */
 };
 
 /*

index 4ac6d48..16606f4 100644
@@ -2,6 +2,7 @@ obj-y				=  mce.o mce-severity.o
 
 obj-$(CONFIG_X86_ANCIENT_MCE)	+= winchip.o p5.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_MCE_XEON75XX)	+= mce-xeon75xx.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
 obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o

index 32996f9..d5b7eec 100644
@@ -28,3 +28,4 @@ extern int mce_ser;
 
 extern struct mce_bank *mce_banks;
 
+extern void (*cpu_specific_poll)(struct mce *);

index a8aacd4..1c1e4aa 100644
@@ -88,6 +88,9 @@ static DECLARE_WAIT_QUEUE_HEAD(mce_wait);
 static DEFINE_PER_CPU(struct mce, mces_seen);
 static int			cpu_missing;
 
+void				(*cpu_specific_poll)(struct mce *);
+EXPORT_SYMBOL_GPL(cpu_specific_poll);
+
 /*
  * CPU/chipset specific EDAC code can register a notifier call here to print
  * MCE errors in a human-readable form.

@@ -364,6 +367,11 @@ static void mce_wrmsrl(u32 msr, u64 v)
 	wrmsrl(msr, v);
 }
 
+static int under_injection(void)
+{
+	return __get_cpu_var(injectm).finished;
+}
+
 /*
  * Simple lockless ring to communicate PFNs from the exception handler with the
  * process context work function. This is vastly simplified because there's

@@ -567,6 +575,10 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 		if (!(flags & MCP_TIMESTAMP))
 			m.tsc = 0;
+
+		if (cpu_specific_poll && !under_injection() && !mce_dont_log_ce)
+			cpu_specific_poll(&m);
+
 		/*
 		 * Don't get the IP here because it's unlikely to
 		 * have anything to do with the actual error location.

index a1a7876..65ebdb6 100644
@@ -79,7 +79,7 @@ EXPORT_SYMBOL_GPL(e820_any_mapped);
  * Note: this function only works correct if the e820 table is sorted and
  * not-overlapping, which is the case
  */
-int __init e820_all_mapped(u64 start, u64 end, unsigned type)
+int e820_all_mapped(u64 start, u64 end, unsigned type)
 {
 	int i;
 

@@ -106,6 +106,7 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(e820_all_mapped);
 
 /*
  * Add a memory region to the kernel e820 map.

Commit-ID:  f91c4d2649531cc36e10c6bc0f92d0f99116b209
Gitweb:     http://git.kernel.org/tip/f91c4d2649531cc36e10c6bc0f92d0f99116b209
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Thu, 21 Jan 2010 18:31:54 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Thu, 21 Jan 2010 18:31:54 -0800

x86, mce: Rename cpu_specific_poll to mce_cpu_specific_poll

cpu_specific_poll is a global variable, and it should have a global namespace name. Since it is MCE-specific (it takes a struct mce *), rename it mce_cpu_specific_poll.

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce-internal.h |    2 +-
 arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c |    4 ++--
 arch/x86/kernel/cpu/mcheck/mce.c          |    8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

index d5b7eec..8e7c2f4 100644
@@ -28,4 +28,4 @@ extern int mce_ser;
 
 extern struct mce_bank *mce_banks;
 
-extern void (*cpu_specific_poll)(struct mce *);
+extern void (*mce_cpu_specific_poll)(struct mce *);

index 67ad39b..e09b736 100644
@@ -396,7 +396,7 @@ static int __init xeon75xx_mce_init(void)
 	pr_info("Found Xeon75xx PFA memory error translation table at %x\n",
 		addr);
 	mb();
-	cpu_specific_poll = xeon75xx_mce_poll;
+	mce_cpu_specific_poll = xeon75xx_mce_poll;
 	return 0;
 
 error_unmap:

@@ -412,7 +412,7 @@ MODULE_DESCRIPTION("Intel Xeon 75xx specific DIMM error reporting");
 #ifdef CONFIG_MODULE
 static void __exit xeon75xx_mce_exit(void)
 {
-	cpu_specific_poll = NULL;
+	mce_cpu_specific_poll = NULL;
 	wmb();
 	/* Wait for all machine checks to finish before really unloading */
 	synchronize_rcu();

index 1c1e4aa..a277d34 100644
@@ -88,8 +88,8 @@ static DECLARE_WAIT_QUEUE_HEAD(mce_wait);
 static DEFINE_PER_CPU(struct mce, mces_seen);
 static int			cpu_missing;
 
-void				(*cpu_specific_poll)(struct mce *);
-EXPORT_SYMBOL_GPL(cpu_specific_poll);
+void				(*mce_cpu_specific_poll)(struct mce *);
+EXPORT_SYMBOL_GPL(mce_cpu_specific_poll);
 
 /*
  * CPU/chipset specific EDAC code can register a notifier call here to print

@@ -576,8 +576,8 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		if (!(flags & MCP_TIMESTAMP))
 			m.tsc = 0;
 
-		if (cpu_specific_poll && !under_injection() && !mce_dont_log_ce)
-			cpu_specific_poll(&m);
+		if (mce_cpu_specific_poll && !under_injection() && !mce_dont_log_ce)
+			mce_cpu_specific_poll(&m);
 
 		/*
 		 * Don't get the IP here because it's unlikely to

FYI, this commit triggered a -tip test failure:

arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c: In function 'xeon75xx_mce_init':
arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c:340: error: implicit declaration of function 'pci_match_id'

I'm excluding it from tip:master.

But the bigger problem with this commit is structure of it - or the lack thereof.

It just blindly goes into the direction the MCE code has been going for some time, minimally enabling the hardware, ignoring both the new EDAC design and the new performance monitoring related design i outlined some time ago.

Newly introduced MCE code in the x86 tree should be integrated much better into existing higher level facilities, not just limp along ignoring life outside of its little box. This will have to be addressed before we can reintroduce this commit, even if this trivial build failure is addressed.

I've Cc:-ed a few more interested parties.

Thanks,

Ingo

(Adding some more interested parties to Cc:)

I completely agree - from what I see this is adding vendor- or rather vendor-and-machine-specific hooks to read out (1) the position of the the memory translation table from PCI config space (0x8c), (2) then to read out the offset from the first MCA status register in order to (3) rdmsr the status information.

In AMD's case, we need similar hooks too, in order to evaluate correctable MCEs for different RAS reasons like for example L3 cache or data arrays errors for disabling L3 indices. I was looking into adding hooks into machine_check_poll() and cpu_specific_poll() interface could work.

Furthermore, lets leave mcheck be mcheck and do error decoding in EDAC modules. For example, there was a core i7 EDAC module submission from Mauro and the Xeon75xx-specific decoding bits could be added to it or even as a new machine-specific module instead of mcelog.

With the evergrowing complexity of memory controller design I don't think that the userspace mcelog approach will scale - you need the whole decoding in the kernel where the module knows the exact memory controllers setup and which DRAM addresses belong to which nodes and whether you do memory hoisting and whether you interleave, if yes, how and on what granilarity you interleave and on and on...

I believe Ingo also had some ideas about perf_event integration and this is something we could add to the MCE polling routine too. Ingo?

Regards/Gruss,
    Boris.

Yep. Could you give a few pointers to Andi where exactly you'd like to see the Intel Xeon functionality added to the EDAC code? There is some Intel functionality there already, but the current upstream code does not look very uptodate. I've looked at e752x_edac.c. (there's some Corei7 work pending, right?) In any case there's a lot of fixing to be done to the Intel code there.

Memory controller functionality integrated into the kernel proper is a good idea partly for similar reasons an on-die memory controller is a good idea in the hardware space.

Yes, my initial thoughts on that are in the lkml mail below from a few months ago. We basically want to enumerate the hardware and its events intelligently
- and integrate that nicely with other sources of events. That will give us a boatload of new performance monitoring and analysis features that we could not have dreamt of before.

Certain events can be 'richer' and 'more special' than others (they can cause things like signals - on correctable memory faults), but so far there's little that deviates from the view that these are all system events, and that we want a good in-kernel enumeration and handling of them. Exposing it on the low level a'la mcelog is a fundamentally bad idea as it pushes hardware complexity into user-space (handling hardware functionality and building good abstractions on it is the task of the kernel - every time we push that to user-space the kernel becomes a little bit poorer).

Note that this very much plugs into the whole problem space of how to enumerate CPU cache hierarchies - something that i think Andreas is keenly interested in. We want one unified enumeration of hardware [and software] components and one enumeration of the events that originate from there.

Right now we are mostly focused on software component enumeration via /debug/tracing/events, but that does not (and should not) remain so. It's not a small task to implement all aspects of that, but it can be done gradually and it will be very rewarding all along the way in my opinion.

[ Furthermore, if there's interest i wouldnt mind a 'perf mce' (or more
generally a 'perf edac') subcommand to perf either, which would specifically be centered about all things EDAC/MCE policy. (but of course other tooling can make use of it too - it doesnt 'have' to be within tools/perf/ per se - it's just a convenient and friendly place for kernel developers and makes it easy to backtest any new kernel code in this area.)

We already have subsystem specific perf subcommands: perf kmem, perf lock, perf sched - this kind of spread out and subsystem specific support it's one of the strong sides of perf. ]

Andi's patch goes in the exact opposite direction of all that, which is a step backwards - and i'm not accepting that. Please at minimum come up with a good drivers/edac/ solution. (and better yet, make it all plug into perf event logging/monitoring and help bring hardware diagnostics and monitoring to all Linux users.)

Thanks,

Ingo

----- Forwarded message from Ingo Molnar <[email protected]> -----

Date: Tue, 22 Sep 2009 15:39:49 +0200
From: Ingo Molnar <[email protected]>
To: Huang Ying <[email protected]>
Cc: Borislav Petkov <[email protected]>,
Fr??d??ric Weisbecker <[email protected]>,
Li Zefan <[email protected]>,
Steven Rostedt <[email protected]>,
"H. Peter Anvin" <[email protected]>, Andi Kleen <[email protected]>, Hidetoshi Seto <[email protected]>, "[email protected]" <[email protected]>, Arjan van de Ven <[email protected]>,
Thomas Gleixner <[email protected]>
Subject: [PATCH] x86: mce: New MCE logging design

TRACE_EVENT() can be used to define the tracepoint itself - but most of the benefit comes from using the performance events subsystem for event reporting. It's a perfect match for MCE events - see below.

The fact that event reporting is used for other things as well might even make it more fault-resilient than open-coded MCE code: just due to it possibly being hot in the cache already (and thus not needing extra DRAM cycles to a possibly faulty piece of hardware) while the MCE code, not used by anything else, has to be brought into the cache for error processing - triggering more faults.

But ... that argument is even largely irrelevant: the overriding core kernel principle was always that of reuse of facilities. We want to keep performance event reporting simple and robust too, so re-using facilities is a good thing and the needs of MCE logging will improve perf events - and vice versa.

Furthermore, to me as x86 maintainer it matters a lot that a piece of code i maintain is clean, well-designed and maintainable in the long run. The current /dev/mcelog code is not clean and not maintainable, and that needs to be fixed before it can be extended.

That MCE logging also becomes more robust and more feature-rich as a result is a happy side-effect.

Well, this is all 2.6.33 stuff and that's plenty of time to do the generic events + perf events approach.

Anyway, i think we can work on these MCE enhancements much better if we move it to the level of actual patches. Let me give you a (very small) demonstration of my requests.

The patch attached below adds two new events to the MCE code, to the thermal throttling code:

mce_thermal_throttle_hot
mce_thermal_throttle_cold

If you apply the patch below to latest -tip and boot that kernel and do:

cd tools/perf/
make -j install

And type 'perf list', those two new MCE events will show up in the list of available events:

  # perf list
  [...]
    mce:mce_thermal_throttle_hot               [Tracepoint event]
    mce:mce_thermal_throttle_cold              [Tracepoint event]
  [...]

Initially you can check whether those events are occuring:

  # perf stat -a -e mce:mce_thermal_throttle_hot sleep 1

Performance counter stats for 'sleep 1':

0 mce:mce_thermal_throttle_hot # 0.000 M/sec

1.000672214 seconds time elapsed

None on a normal system. Now, i tried it on a system that gets frequent thermal events, and there it gives:

  # perf stat -a -e mce:mce_thermal_throttle_hot sleep 1

Performance counter stats for 'sleep 1':

              73  mce:mce_thermal_throttle_hot #      0.000 M/sec

     1.000765260  seconds time elapsed

Now i can observe the frequency of those thermal throttling events:

  # perf stat --repeat 10 -a -e mce:mce_thermal_throttle_hot sleep 1

Performance counter stats for 'sleep 1' (10 runs):

             184  mce:mce_thermal_throttle_hot #      0.000 M/sec   ( +-   1.971% )

     1.006488821  seconds time elapsed   ( +-   0.203% )

184/sec is pretty hefty, with a relatively stable rate.

Now i try to see exactly which code is affected by those thermal throttling events. For that i use 'perf record', and profile the whole system for 10 seconds:

  # perf record -f -e mce:mce_thermal_throttle_hot -c 1 -a sleep 10
  [ perf record: Woken up 4 times to write data ]
  [ perf record: Captured and wrote 0.802 MB perf.data (~35026 samples) ]

And i used 'perf report' to analyze where the MCE events occured:

# Samples: 2495
#
# Overhead Command Shared Object Symbol

 # ........  ..............  ........................  ......
 #
    55.39%       hackbench  [vdso]                    [.] 0x000000f77e1430
    35.27%       hackbench                  f77c2430  [.] 0x000000f77c2430
     3.05%       hackbench  [kernel]                  [k] ia32_setup_sigcontext
     0.88%            bash  [vdso]                    [.] 0x000000f776f430
     0.64%            bash                  f7731430  [.] 0x000000f7731430
     0.52%       hackbench  ./hackbench               [.] sender
     0.48%            nice                  f7753430  [.] 0x000000f7753430
     0.36%       hackbench  ./hackbench               [.] receiver
     0.24%            perf  /lib64/libc-2.5.so        [.] __GI_read
     0.24%       hackbench  /lib/libc-2.5.so          [.] __libc_read

Most of the events concentrate themselves to around syscall entry points. That's not because the system is doing that most of the time - it's probably because the fast syscall entry point is a critical path of the CPU and overheating events likely concentrate themselves to around that place.

Now lets how we can do very detailed MCE logs, with per CPU data, timestamps and other events mixed. For that i use perf record again:

# perf record -R -f -e mce:mce_thermal_throttle_hot:r -e mce:mce_thermal_throttle_cold:r -e irq:irq_handler_entry:r -M -c 1 -a sleep 10
[ perf record: Woken up 12 times to write data ]
[ perf record: Captured and wrote 1.167 MB perf.data (~50990 samples) ]

And i use 'perf trace' to look at the result:

       hackbench-5070  [000]  1542.044184301: mce_thermal_throttle_hot: throttle_count=150852
       hackbench-5070  [000]  1542.044672817: mce_thermal_throttle_cold: throttle_count=150852
       hackbench-5056  [000]  1542.047029212: mce_thermal_throttle_hot: throttle_count=150853
       hackbench-5057  [000]  1542.047518733: mce_thermal_throttle_cold: throttle_count=150853
       hackbench-5058  [000]  1542.049822672: mce_thermal_throttle_hot: throttle_count=150854
       hackbench-5046  [000]  1542.050312612: mce_thermal_throttle_cold: throttle_count=150854
       hackbench-5072  [000]  1542.052720357: mce_thermal_throttle_hot: throttle_count=150855
       hackbench-5072  [000]  1542.053210612: mce_thermal_throttle_cold: throttle_count=150855
           :5074-5074  [000]  1542.055689468: mce_thermal_throttle_hot: throttle_count=150856
       hackbench-5011  [000]  1542.056179849: mce_thermal_throttle_cold: throttle_count=150856
           :5061-5061  [000]  1542.056937152: mce_thermal_throttle_hot: throttle_count=150857
       hackbench-5048  [000]  1542.057428970: mce_thermal_throttle_cold: throttle_count=150857
       hackbench-5053  [000]  1542.060372255: mce_thermal_throttle_hot: throttle_count=150858
       hackbench-5054  [000]  1542.060863006: mce_thermal_throttle_cold: throttle_count=150858

The first thing we can see it from the trace, only the first core reports thermal events (the box is a dual-core box). The other thing is the timing: the CPU is in 'hot' state for about ~490 microseconds, then in 'cold' state for ~250 microseconds.

So the CPU spends about 65% of its time in hot-throttled state, to cool down enough to switch back to cold mode again.

Also, most of the overheating events hit the 'hackbench' app. (which is no surprise on this box as it is only running that workload - but might be interesting on other systems with more mixed workloads.)

So even in its completely MCE-unaware form, and with this small patch that took 5 minutes to write, the performance events subsystem and tooling can already give far greater insight into characteristics of these events than the MCE subsystem did before.

Obviously the reported events can be used for different, MCE specific things as well:

- Report an alert on the desktop (without having to scan the syslog all the time)

- Mail the sysadmin

- Do some specific policy action such as soft-throttle the workload until the situation and the log data can be examined.

As i outlined it in my previous mail, the perf events subsystem provides a wide range of facilities that can be used for that purpose.

If there are aspects of MCE reporting usecases that necessiate the extension of perf events, then we can do that - both sides will benefit from it. What we dont want to do is to prolongue the pain caused by the inferior, buggy and poorly designed /dev/mcelog ABI and code. It's not serving our users well and people are spending time on fixing something that has been misdesigned from the get go instead of concentrating on improving the right facilities.

So this is really how i see the future of the MCE logging subsystem: use meaningful generic events that empowers users, admins and tools to do something intelligent with the available stream of MCE data. The hardware tells us a _lot_ of information - lets use and expose that in a structured form.

Ingo

---
 arch/x86/kernel/cpu/mcheck/therm_throt.c |    9 +++++
 include/trace/events/mce.h               |   47 +++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/cpu/mcheck/therm_throt.c
===================================================================
@@ -31,6 +31,9 @@
 #include <asm/mce.h>
 #include <asm/msr.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/mce.h>
+
 /* How long to wait between reporting thermal events */
 #define CHECK_INTERVAL		(300 * HZ)
 

@@ -118,8 +121,12 @@ static int therm_throt_process(bool is_t
 	was_throttled = state->is_throttled;
 	state->is_throttled = is_throttled;
 
-	if (is_throttled)
+	if (is_throttled) {
 		state->throttle_count++;
+		trace_mce_thermal_throttle_hot(state->throttle_count);
+	} else {
+		trace_mce_thermal_throttle_cold(state->throttle_count);
+	}
 
 	if (time_before64(now, state->next_check) &&
 			state->throttle_count != state->last_throttle_count)

----- End forwarded message -----

On Sat, Jan 23, 2010 at 10:00:03AM +0100, Ingo Molnar wrote:

[..]

Basically you've named them all - I'd go for a new module/c file though if the Xeon75 stuff is completely new hw and cannot reuse existing EDAC modules.

Oh yes, he's interested in that allright :)

Yes, this is very interesting. How do we represent that in the kernel space as one contiguous "tree" or "library" or whatever without adding overhead and opening that info to userspace?

Because this is one thing that has been bugging us for a long time. We don't have a centralized smart utility with lots of small subcommands like perf or git, if you like, which can dump you the whole or parts of the hw configuration of the machine - something like cache sizes and hierarchy, CPU capabilities from CPUID flags, memory controllers configuration, DRAM type and sizes, NUMA info, processor PCI config space along with decoded register and bit values, ... (where do I stop)...

Currently, we have a ragged collection of tools with their own syntax and output formatting like numactl, x86info, /proc/cpuinfo, (eyeballing dmesg output - which is not even a tool :) and it is very annoying when you have a bunch of machines and you start pulling them tools in, one after another, before you can even get to the hw information.

So, it would be much much more useful if we had such a tool that can give you a precise hw information without disrupting the kernel (I remember several bugs with ide-cd last year where some udev helpers were querying the drive for capabilities but the drive wasn't ready yet and, as a result, was getting puzzled so much that it wouldn't load properly). Its subcommands could each cover a subsystem or a hw component and you could do something like the following example (values in {} are actual settings read from the hardware):

<tool> pcicfg -f 18.3 -r 0xe8
F3x0e8 (Northbridge Capabilities Register): {0x02073f99}

...

L3Capable: [25]: {1}
1=Specifies that an L3 cache is present. See CPUID Fn8000_0006_EDX.

...

LnkRtryCap: [11]: {1}
Link error-retry capable.
HTC_capable: [10]: {1}
This affects F3x64 and F3x68.
SVM_capable: [9]: {1}

MctCap: [8]: {1}
memory controller (on the processor) capable.
DdrMaxRate: [7:5]: {0x4}

                                Specifies the maximum DRAM data rate that the
                                processor is designed to support.
                                Bits 	DDR limit  		Bits    DDR limit
				====	=========		====	=========
				000b	No limit		100b	800 MT/s
				001b	Reserved		101b	667 MT/s
				010b	1333 MT/s		110b	533 MT/s
				011b	1067 MT/s		111b	400 MT/s

Chipkill_ECC_capable: [4]: {1}

ECC_capable: [3]: {1}

Eight_node_multi_processor_capable: [2]: {0}

Dual_node_multi_processor_capable: [1]: {0}

DctDualCap: [0]: {1}
two-channel DRAM capable (i.e., 128 bit). 0=Single channel (64-bit) only.

And yes, this is very detailed output but it simply serves the purpose to show how detailed we can get.

The same thing can output MSR registers like lsmsr does:

MC4_CTL = 0x000000003fffffff (CECCEn=0x1, UECCEn=0x1, CrcErr0En=0x1, CrcErr1En=0x1, CrcErr2En=0x1, SyncPkt0En=0x1, SyncPkt1En=0x1, SyncPkt2En=0x1, MstrAbrtEn=0x1, TgtAbrtEn=0x1, GartTblWkEn=0x1, AtomicRMWEn=0x1, WDTRptEn=0x1, DevErrEn=0x1, L3ArrayCorEn=0x1, L3ArrayUCEn=0x1, HtProtEn=0x1, HtDataEn=0x1, DramParEn=0x1, RtryHt0En=0x1, RtryHt1En=0x1, RtryHt2En=0x1, RtryHt3En=0x1, CrcErr3En=0x1, SyncPkt3En=0x1, McaUsPwDatErrEn=0x1, NbArrayParEn=0x1, TblWlkDatErrEn=0x1)

but with in a more human-readable form without the need to open the hw manual for that. And this is pretty lowlevel. How about nodes and cores on each node and HT siblings and NUMA proximity and DIMM distribution across NBs and which northbridge is connected to to the southbridge on a multinode system, etc? I know, we have parts of that in /sysfs but it should be easier to get that info.

You can have a gazillion examples like those and the use cases are not a small number: ask a user for a specific hw configuration when debugging, output from this tool can do automatic tuning suggestions like powertop in 'perf stat' runs where the machine spends too much time in a function because, for example, the HT link has been configured to a lower speed for power savings but the app that is being profiled is generating a bunch of threads doing parallel computations and causing a bunch of cross-node traffic which slows it down, etc. etc. etc.

The example below (which I cut for brevity) is a perfect example of how it should be done. Let me first, however, go a step back and give you my opinion of how I think this whole MCEs catching and decoding should be done before we think of tooling:

1. We need to notify userspace, as you've said earlier, and not scan the syslog all the time. And EDAC, although decoding the correctable ECC, spews it in the syslog too causing more parsing (there's edac-utils which polls /sysfs but this is just another tool with problems as outlined above).

What is more, the notification mechanism we come up with should push the error as early as possible and be able to send it over the network to a monitor (think data center with thousands of compute nodes here where CECCs happen every day at least) - something like a more resilient netconsole which sends out decoded MCE info to the monitor.

2. Also another very good point you had is go into maintenance mode by throttling or even suspend all uspace processes and start a restricted maintenance shell after an MCE happens. This should be done based on the severity of the MCE and the shell should run on a core that _didn't_ observe the MCE.

3. All the hw events like correctable ECCs should be thresholded so that all errors exceeding a preset threshold (below that is normal operation and they get corrected by ECC codes in the hardware anyway) should alarm of a slowly failing DIMM or a L3 subcache index for the sysop to take action against if the machine cannot do failover itself. For example, in the L3 cache case, the machine can initially disable max. 2 subcache indices and notify the user that it has done so but the user should be warned that the hw is failing slowly.

The current decoding needs more loving too since now it says something like the following:

EDAC DEBUG: in drivers/edac/amd64_edac_inj.c, line at 170: section=0x80000002 word_bits=0x10020001 EDAC DEBUG: in drivers/edac/amd64_edac_inj.c, line at 170: section=0x80000002 word_bits=0x10020001
Northbridge Error, node 0, core: -1
K8 ECC error.
EDAC amd64 MC0: CE ERROR_ADDRESS= 0x33574910
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1572: (dram=0) Base=0x0 SystemAddr= 0x33574910 Limit=0x12fffffff
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1583: HoleOffset=0x3000 HoleValid=0x1 IntlvSel=0x0
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1627: (ChannelAddrLong=0x19aba480) >> 8 becomes InputAddr=0x19aba4
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1515: InputAddr=0x19aba4 channelselect=0
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1537: CSROW=0 CSBase=0x0 RAW CSMask=0x783ee0
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1541: Final CSMask=0x7ffeff
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1544: (InputAddr & ~CSMask)=0x100 (CSBase & ~CSMask)=0x0
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1537: CSROW=1 CSBase=0x100 RAW CSMask=0x783ee0
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1541: Final CSMask=0x7ffeff
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1544: (InputAddr & ~CSMask)=0x100 (CSBase & ~CSMask)=0x100
EDAC DEBUG: in drivers/edac/amd64_edac.c, line at 1549: MATCH csrow=1
EDAC MC0: CE page 0x33574, offset 0x910, grain 0, syndrome 0xbe01, row 1, channel 0, label "": amd64_edac EDAC MC0: CE - no information available: amd64_edacError Overflow
EDAC DEBUG: in drivers/edac/amd64_edac_inj.c, line at 170: section=0x80000002 word_bits=0x10020001

and this is only the chip select row but we need to map that to the actual DIMM and to tell the admin: "DIMM with label "BLA" on your motherboard seems to be failing" without first naming all DIMMs through /sysfs to their silk-screen labels.

And yes, it is a lot of work but we can at least start talking about it and gradually getting it done. What do the others think?

Thanks.

Regards/Gruss,
    Boris.

Hi,

PC configuration is all in dmidecode, CPU/node information in lscpu these days (part of utils-linux)

The dmidecode information could be perhaps presented nicer, but I don't think we need any fundamental new tools.

mcelog never scanned the syslog all the time. This is just EDAC misdesign.

But yes syslog is exactly the wrong interface for these kinds of errors.

When you have a unrecoverable MCE this is not safe because you can't write anything to disk (and usually the system is unstable and will crash soon) because there are uncontained errors somewhere in the hardware. The most important thing to do in this situation is to *NOT* write anything to disk (and that is the reason why the hardware raised the unrecoverable MCE in the first place) Having a shell without being able to write to disk doesn't make sense.

When you have a recoverable MCE with contained errors this is not needed, because it, well, just recovers.

Agreed. Corrected errors without thresholds are useless (that is one of the main reasons why syslog is a bad idea for them)

See also my plumbers presentation on the topic:

http://halobates.de/plumbers-error.pdf

One key part is that for most interesting reactions to thresholds you need user space, kernel space is too limited.

My current direction was implementing this in mcelog which maintains threshold counters and already does a couple of direct (user based) threshold reactions, like offlining cores and pages and reporting short user friendly error summaries when thresholds are exceeded.

Longer term I hope to move to a more generic (user) error infrastructure that handles more kinds of errors. This needs some infrastructure work, but not too much.

Yes, see the slide set above on thoughts how a good error looks like.

The big problem with EDAC currently is that it neither gives the information actually needed (like mainboard labels), but gives a lot of irrelevant low level information. And since it's kernel based it cannot do most of the interesting reactions. And it doesn't have a usable interface to add user events.

And yes having all that crap in syslog is completely useless, unless you're debugging code.

-Andi

[email protected] -- Speaking for myself only.

Hi,

Uuh, dmidecode doesn't even start to look usable in my book because you have to rely on BIOS vendors to fill out the information for you. Here are some assorted excerpts from dmidecode on my machines:

1. Incomplete info:

Handle 0x0001, DMI type 1, 27 bytes
System Information
Manufacturer: System manufacturer Product Name: System Product Name
Version: System Version
Serial Number: System Serial Number
UUID: 201EE116-055E-DD11-8B0E-002215FDB1C6 Wake-up Type: Power Switch
SKU Number: To Be Filled By O.E.M.
Family: To Be Filled By O.E.M.

2. Wrong(!) info:

Handle 0x0007, DMI type 7, 19 bytes
Cache Information
Socket Designation: L3-Cache
Configuration: Enabled, Not Socketed, Level 3 Operational Mode: Varies With Memory Address
Location: Internal
Installed Size: 6144 KB
Maximum Size: 6144 KB
Supported SRAM Types:
Pipeline Burst
Installed SRAM Type: Pipeline Burst
Speed: Unknown

why?

Error Correction Type: Single-bit ECC
System Type: Unified
Associativity: 4-way Set-associative

how is my L3 4-way set-associative and how do they come up with that???

[ by the way, it says the same on my old P4 box' L2 so this could mean
anything besides the actual cache assoc. ]

Here's what the dmidecode manpage says:

"...

BUGS
More often than not, information contained in the DMI tables is inaccurate, incomplete or simply wrong.
...
"

so I guess I'm not the only one :)

In the end, even if the info were correct, it is still not nearly enough for all the information you might need from a system. So you end up pulling a dozen of different tools just to get the info you need. So yes, I really do think we need a tool to get do the job done right and on any system. And this tool should be distributed with the kernel sources like perf is, so that you don't have to jump through hoops to pull the stuff (Esp. if you have to build everything everytime like Andreas does :)).

Oh yes, EDAC has the edac-utils too which access /sysfs files but even so, it is suboptimal and we really need a single interface/output channel/whatever you call a beast like that to reliably transfer human readable hw error info to userspace and/or network. And this has to be pushed from kernel space outwards as early as the gravity of the error suggests, IMO.

Agreed completely.

Hmm, not necessarily. First of all, not all UC errors are absolutely valid reasons to panic the machine. Imagine, for example, you encounter (as unlikely as it might be) a multibit error during L1 data cache scrubbing which hasn't been consumed yet. Now, technically, no data corruption has taken place yet so you can easily start the shell on another core which doesn't have that datum in its cache, decode the error for the user to see what it was and even allow her/him to poweroff the machine properly.

Or imagine you have a L2 TLB multimatch - also UC but you can still invalidate the two entries, maybe kill the processes that have caused those mappings and start the shell.

So no, not all UC errors have to absolutely cause data corruption and you can still prepare for a clean exit by warning the user that her/his data might be compromized and whether (s)he wants to write to disk or poweroff the machine immediately SysRq-O style.

And even if an UC causes data corruption, panicking the system doesn't mean that the error has been contained. Nothing can assure you that by the time do_machine_check() has run the corrupted data hasn't left the CPU core and landed in another core's cache (maybe even on a different node) and then on disk through an outstanding write request. That's why we syncflood the HT links on certain error types since an MCE is not enough to stop that propagation.

Yep, sounds good.

Yep, I think this is something we should definitely talk about since our error reporting right now needs a bunch of work to even start becoming really usable.

Yes, I'm very well aware of that. I'm currently working on a solution. It's just an idea now but I might be able to read DIMM configuration on the SPD ROM on the DIMM along with their labels and position on the motherboard in order to be able to pinpoint the correct DIMM... Stay tuned...

So basically, IMHO we need:

1. Resilient error reporting that reliably pushes decoded error info to userspace and/or network. That one might be tricky to do but we'll get there.

2. Error severity grading and acting upon each type accordingly. This might need to be vendor-specific.

3. Proper error format suiting all types of errors.

4. Vendor-specific hooks where it is needed for in-kernel handling of certain errors (L3 cache index disable, for example).

5. Error thresholding, representation, etc all done in userspace (maybe even on a different machine).

6. Last but not least, and maybe this is wishful thinking, a good tool to dump hwinfo from the kernel. We do a great job of detecting that info already - we should do something with it, at least report it...

Let's see what the others think.

Thanks.

Regards/Gruss,
    Boris.

How about having a system file which can be maintained with kernel, e.g. like /proc/hwinfo, /sys/devices/platform/hwinfo, or directory with some files like /somewhere/hwinfo/{dmi,acpi,pci,...} etc.?

I think it would be better to think "error" is a subset of "event", which could be reported if interested but otherwise be filtered. Use of TRACE_EVENT() for mce event aim such approach at least.

I think you mean severity grading in kernel.
Even if hardware reported an error and graded it as corrected, kernel can escalate the severity, likely based on some threshold.

As mentioned in Andi's PDF, CPER format is one of good candidate available today, I think.
However we could invent more suitable one if needed.

Some difficulty would be there to add such hook in the UE handling path, but anyway we can have it for the CE path. Just need to be organized.

(...BTW, how about putting mcelog tree under the /tools, Andi?)

Of course I want to have a tool to get a summary (not full dump) of current hardware status too: e.g.

  $ cat ./hwinfo/faulty
  WARN: DIMM @ slot X on node Y: 208 errors corrected in last 3 days
  INFO: PCI 0000:NN:01.1: 1 error recovered 37 hours ago

Thanks,
H.Seto

Why not do that in user space?

In fact it's often already done.

Just because we're kernel programmers doesn't mean that everything needs to be solved inside the kernel.

The whole trace event infrastructure right now is not really aimed/useful for "always on low overhead background monitoring" like standard error handling requires.

In principle it could be probably fixed (although I'm a bit sceptical on the "low overhead" part), but I suspect the result would be neither optimized for error handling nor optimized for performance monitoring anymore. They simply have very different requirements.

When you do full event tracing anyways it makes some sense to get events for errors too, but that's a quite different use-case.

For the "standard" error handling I think we're better of with something optimized for the job.

I don't think the kernel should do that, it's so much a policy decision and these are best kept as near the administrator as possible (= user space)

That is for some cases it might make sense to have limited thresholds in the kernel, but I suspect they are limited. Mostly it would be the case when the hardware itselfs already keeps these counters.

Yes for hardware errors. It's definitely not perfect and somewhat overdesigned, but I'm not sure we could come up with a much better one. At least a subset of it with some extensions might do. Also in some cases the error is already in this format.

The advantage of it is that it's at least well understood and documented.

I don't see the advantage. Linux has always been a collection of packages, not a unified single big tree. Also my current impression is that the in tree user space builds don't work very well.

-Andi

[email protected] -- Speaking for myself only.

For most of the information in DMI decode you can't even find it any other way. If you can't get it from the BIOS it's simply not there.

One example are silkscreen labels, which are extremly important for any kind of hardware error handling.

On my server class systems the information is largely correct. If it's not on your system perhaps you need to complain to the vendor.

On non server class there are a lot of BIOS problems, but typically the platforms there don't have enough support for good error handling anyways.

Cache/CPU information is in lscpu. The important part are the motherboard resources.

Most distributions have some kind of summary tool to aggregate complete system configuration.

It's not the same everywhere, but that's one of the strengths of Linux imho, not a weakness.

You just reinvented mcelog, congratulations.

When no data corruption has been taken it's not a UC error.

An UC error in this case is defined as something that the hardware tells us is a UC error, worse even a uncontained UC error

The reason the hardware tells us about that is that it wants us to prevent further damage. And the primary way to do that is to NOT write anything to disk, because that would risk corrupting it.

For contained memory UC errors I wrote all the infrastructure last year to handle them. Making hwpoison better is still an ongoing project, but it's already quite usable.

That is why the panic stops all cpus to prevent that.

But yes if the disk write happens at exactly the wrong point the error could still escape, but we try to keep the window as small as possible. Typically there's also some hardware help with that to catch currently in flight transactions. It depends on the platform how well it works.

The SPD ROM doesn't have labels. The only entity who knows them is the BIOS (or someone who just studied the semantics of the motherboard, but I don't think we can rely on that)

Not at all tricky. At least on modern Intel platforms mcelog already does it.

mcelog does it mostly. It's not perfect yet, but not too bad.

I plan to look into that.

But "suiting all types of errors" is probably a mistake, I don't think it makes sense to try to invent the one perfect error that covers everything. People have tried that in the past and it was always a spectacular failure.

I suspect the better goal is rather a range of error formats for common situations with a lot of flexibility.

mcelog does that for memory errors on modern systems.

IMHO there are already enough of them.

-Andi

[email protected] -- Speaking for myself only.

If you do it within perf, or at least share code with it, you'd basically use its accessor methods as a library. There's parsers for event descriptors, so if the kernel exposes events it's all rather straightforward to code for.

For more dynamic set of events we could enhance the ftrace event interface some more as well - or possibly create a separate 'events' filesystem that enumerates all sorts of event sources in some nice unified hierarchy.

This is something that has come up before in other contexts as well.

Via perf events you can get a super-fast mmap()-ed ring-buffer and get the info in a lightweight way and possibly squeeze in some policy action before the system possibly dies.

Yep.

I like it.

You can do it as a 'perf hw' subcommand - or start off a fork as the 'hw' utility, if you'd like to maintain it separately. It would have a daemon component as well, to receive and log hardware events continuously, to trigger policy action, etc.

I'd suggest you start to do it in small steps, always having something that works - and extend it gradually.

Ingo

From: Ingo Molnar <[email protected]>
Date: Tue, Feb 16, 2010 at 10:02:15PM +0100
Hi,

I had the chance to meditate over the weekend a bit more on the whole RAS thing after rereading all the discussion points more carefully. Here are some aspects I think are important which I'd like to drop here rather sooner than later so that we're in sync and don't waste time implementing the wrong stuff:

* Critical errors: we need to switch to a console and dump decoded error there at least, before panicking. Nowadays, almost everyone has a camera with which that information can be extracted from the screen. I'm afraid we won't be able to send the error over a network since climbing up the TCP stack takes relatively long and we cannot risk error propagation...? We could try to do it on a core which is not affected by the error though as a last step in the sequence...

I think this is much more user-friendly than the current panicking which is never seen when running X except when the user has a serial/netconsole sending to some other machine.

All other non-that-critical errors are copied to userspace over a mmapped buffer and then the uspace daemon is being poked with a uevent to dump the error/signal over network/parse its contents and do policy stuff.

* receive commands by syscall, also for hw config: I like the idea of sending commands to the kernel over a syscall, we can reuse perf functionality here and make those reused bits generic.

* do not bind to error format etc: not a big fan of slaving to an error format - just dump error info into the buffer and let userspace format it. We can do the formatting if we absolutely have to.

* can also configure hw: The tool can also send commands over the syscall to configure certain aspects of the hardware, like:

- disable L3 cache indices which are faulty
- enable/disable MCE error sources: toggle MCi_CTL, MCi_CTL_MASK bits
- disable whole DIMMs: F2x[1, 0][5C:40][CSEnable]
- control ECC checking
- enable/disable powering down of DRAM regions for power savings
- set memory clock frequency
- some other relevant aspects of hw/CPU configuration

* keep all info in sysfs so that no tool is needed for accessing it, similar to ftrace: All knobs needed for user interaction should appear redundantly as sysfs files/dirs so that configuration/query can be done "by hand" even when the hw tool is missing

* gradually move pieces of RAS code into kernel proper: important codepaths/aspects from the HW which are being queried often (e.g., DIMM population and config) should be moved gradually into the kernel proper.

Anyways, this is by all means not complete and still as alpha as it can be. However, I'd like to discuss it as early as possble and in small, incremental steps, omitting trial and error as much as possible. So, feel free to throw all your crazy ideas at me and correct (or kill) all those crappy points above.

Thanks.

Regards/Gruss,
    Boris.

Yep.

If you use perf here you get the events and can poll() the event channel. User-space can decide which events to listen in on. uevent/user-notifier is a bit clumsy for that.

If you use perf and tracepoints to shape the event log format then this is all taken care of already, you get structured event format descriptors in /debug/tracing/events/*. For example there's already an MCE tracepoint in the upstream kernel today (for thermal events):

phoenix:/home/mingo> cat /debug/tracing/events/mce/mce_record/format
name: mce_record
ID: 28
format:

	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;
	field:int common_lock_depth;	offset:8;	size:4;	signed:1;

	field:u64 mcgcap;	offset:16;	size:8;	signed:0;
	field:u64 mcgstatus;	offset:24;	size:8;	signed:0;
	field:u8 bank;	offset:32;	size:1;	signed:0;
	field:u64 status;	offset:40;	size:8;	signed:0;
	field:u64 addr;	offset:48;	size:8;	signed:0;
	field:u64 misc;	offset:56;	size:8;	signed:0;
	field:u64 ip;	offset:64;	size:8;	signed:0;
	field:u8 cs;	offset:72;	size:1;	signed:0;
	field:u64 tsc;	offset:80;	size:8;	signed:0;
	field:u64 walltime;	offset:88;	size:8;	signed:0;
	field:u32 cpu;	offset:96;	size:4;	signed:0;
	field:u32 cpuid;	offset:100;	size:4;	signed:0;
	field:u32 apicid;	offset:104;	size:4;	signed:0;
	field:u32 socketid;	offset:108;	size:4;	signed:0;
	field:u8 cpuvendor;	offset:112;	size:1;	signed:0;

print fmt: "CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, ADDR/MISC: %016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x", REC->cpu, REC->mcgcap, REC->mcgstatus, REC->bank, REC->status, REC->addr, REC->misc, REC->cs, REC->ip, REC->tsc, REC->cpuvendor, REC->cpuid, REC->walltime, REC->socketid, REC->apicid

tools/perf/util/trace-event-parse.c contains the above structured format descriptor parsing code, and can turn it into records that you can read out from C code - and provides all sorts of standard functionality over it.

I'd strongly suggest to reuse that - we _really_ want health monitoring and general system performance monitoring to share a single facility: as they are both one and the same thing, just from different viewpoints.

In other words: 'system component failure' is another metric of 'system performance', so there's strong synergies all around.

Once the hardware's structure is enumerated (into a tree/hiearchy), and events are attached to individual components, then 'commands' are the next logical
step: they are methods of a given component/object.

One such method could be 'injection' functionality btw: to simulate rare hardware failures and to make sure policy logic is ready for all eventualities.

But ... while that is clearly the 'big grand' end goal, the panacea of RAS design, i'd suggest to start with a small but useful base and pick up low hanging fruits - then work towards this end goal. This is how perf is developed/maintained as well.

So i'd suggest to start with _something_ that other people can try and have a look at and extend, for example something that replaces basic mcelog functionality. That alone should be fairly easy and immediately gives it a short-term purpose. It would also be highly beneficial to the x86 code to get rid of the mcelog abonimation.

Please share this code with perf. Profiling needs the same kind of 'hardware structure' enumeration - combined with 'software component enumeration'.

Currently we have that info /debug/tracing/events/. Some hw structure is in there as well, but not much - most of it is kernel subsystem event structure.

sysfs would be an option but IMO it's even better to put ftrace's /debug/tracing/events/ hiearchy into a separate eventfs - and extend it with 'hardware structure' details.

This would not only crystalise the RAS purpose, but would nicely extend perf as well. With every hardware component you add from the RAS angle we'd get new events for tracing/profiling use as well - and vice versa. There's no reason why RAS should be limited to hw component failure events: a RAS policy action could be defined over OOM events too for example, or over checksum failures in network packets - etc.

RAS is not just about hardware, and profiling isnt just about software. We want event logging to be a unified design - there's big advantages to that.

So please go for an integrated design. The easiest and most useful way for that would be to factor out /debug/tracing/events/ into /eventfs.

Yeah. Good plans.

Ingo

If the user has set up kexec/kdump, currently, the error is dumped to the crash log at the local machine or to another machine, depending on the config.

I like the idea of switching to console before panicking, but this is sometimes problematic, as there are some video drivers that switching to console don't always work. Also, this could interfere at the kdump setup, if configured. So, if kdump or serial/netconsole is enabled, the better is to not try to switch the video mode. So, I think the better is to allow userspace to select one or another mode.

It seems interesting, but, if the userspace daemon is not running, it should fallback to write the errors via dmesg.

Why? I think we should keep using sysfs for hw config, and it seems that you also agree. Sysfs work fine and fits nice for enumerating hierarchical data. What's the rationale to add also a syscall API?

The EDAC data model needs some discussion, as, currently, the memory is represented per csrow, and modern MCU don't allow such level of control (and it doesn't make much sense on representing this way, as you can't replace a csrow). The better is to use DIMM as the minumum unit.

The format code is needed for critical errors anyway.

Agreed.

I agree that configuring the hw is interesting, while I still think that we should use sysfs for it.

The i7core, amd64 and a very few other EDAC drivers already implements memory error injection errors, via sysfs. The point with error injection is that this feature is hardware dependent.

So, while you can use the same hierarchy/tree for hw description, things like error injection will require an specific hierarchy. That's one of the reasons why I think we should keep using sysfs: it can easily be used to represent data that are hardware dependent.

The way I mapped this on the i7core_edac is that I kept using the standard EDAC sysfs hierarchy for memory, and added a generic code that allows describing the error injection bits found in Nehalem.

Cheers,
Mauro

Just to start the data model, this is what a typical EDAC driver presents:

/sys/devices/system/edac/mc/mc0/

|-- ce_count                                                                    
|-- ce_noinfo_count                                                             
|-- csrow0
|   |-- ce_count
|   |-- ch0_ce_count
|   |-- ch0_dimm_label
|   |-- ch1_ce_count
|   |-- ch1_dimm_label
|   |-- ch2_ce_count
|   |-- ch2_dimm_label
|   |-- ch3_ce_count
|   |-- ch3_dimm_label
|   |-- dev_type
|   |-- edac_mode
|   |-- mem_type
|   |-- size_mb
|   `-- ue_count
|-- csrow1                                                                      
|   |-- ce_count
|   |-- ch0_ce_count
|   |-- ch0_dimm_label
|   |-- ch1_ce_count
|   |-- ch1_dimm_label
|   |-- ch2_ce_count
|   |-- ch2_dimm_label
|   |-- ch3_ce_count
|   |-- ch3_dimm_label
|   |-- dev_type
|   |-- edac_mode
|   |-- mem_type
|   |-- size_mb
|   `-- ue_count
|-- device -> ../../../../pci0000:3f/0000:3f:03.0                               
|-- mc_name                                                                     
|-- reset_counters                                                              
|-- sdram_scrub_rate                                                            
|-- seconds_since_reset                                                         
|-- size_mb                                                                     
|-- ue_count                                                                    

`-- ue_noinfo_count

In the case of i7core_edac, there's no way to identify csrows by using the public registers (I've no idea is is there any non-documented register for it). So, the driver maps one dimm per "edac csrow".

It would be good to see a better struct for it.

With respect to error injection, this is the way i7core maps it:

|-- inject_addrmatch                                                            
|   |-- bank                                                                    
|   |-- channel                                                                 
|   |-- col                                                                     
|   |-- dimm                                                                    
|   |-- page                                                                    
|   `-- rank                                                                    
|-- inject_eccmask                                                              
|-- inject_enable                                                               
|-- inject_section                                                              
|-- inject_type                                                                 

The inject_addrmatch leaves control a match filter for the error where the error will be inject. I doubt we would find a way to standardize it.

Cheers,
Mauro

First I suspect the result wouldn't be too compatible with current EDAC, so it might be less confusing to give it a new name.

Some thoughts on this:

One of my goals would be that a fall back driver can create the information just from the SMBIOS (that is without error counts, although there are some cases those can be mapped from corrected MCEs)

I think I would prefer a flat model:

socket <-> DIMM

socket could be arbitary, either a CPU socket, or a external memory controller

and then have a "path" string per DIMM that can be a arbitrary path to the DIMM, depending on the system: e.g. on a system with on board buffers it could include the buffer, the channel to the buffer and the channel behind the buffer. The string could be free form like a path name e.g.

FOO-CH1/DDR-CH2/DIMMa

I think it doesn't make sense to have that in the file system hierarchy itself (that would just make it harder to parse without any real benefit, since hardware changes could completely change the directory structure), I would just keep it as an attribute file in the leaf nodes.

I don't think we it makes much sense to have anything below the DIMM (e.g. ranks) These are typically hard to get in a generic way and the most important information on this level is what units to replace.

Then the other important part is a good way to manage fall back counters when you cannot identify the target DIMM (that happens occasionally for various reasons).

In this case would need a "aggregator object" with a truncated path that still maintains the same counts.

Then of course there still needs to be a event interface to let someone actually do something with all these counts without having to poll.

Most interesting cases always need user space support, so also of course user space needs raw events too.

-Andi

[email protected] -- Speaking for myself only.

The i7core_edac driver covers the Nehalem chipsets. It is at:
http://git.kernel.org/?p=linux/kernel/git/mchehab/i7core.git

The error decoding is done by i7core_mce_output_error(). If Xeon 75xx needs to do something different, it is just a matter of calling a function there, to retrieve the values from the proper registers.

In order to avoid duplication at the code, the driver is receiving the error events from the mcelog driver, and having two drivers fighting to access the same error registers at the same time, this driver relies on mce driver to send the events.

I expect to allocate some time to put the driver into a better shape for upstream submission soon.

I agree. The EDAC module handles memory errors for several memory controllers, including some on non-x86 architectures. Letting this decode to happen outside the kernel, just for a very few set of memory controllers, seems a bad idea.

Cheers,
Mauro.

Erm no, first Nehalem isn't a chipset and then it only covers parts of the space (some subset of Nehalems with a specific uncore).

This does not include Xeon 75xx where all this is quite different.

Again you can't do all the things mcelog does in kernel space only.

-Andi

[email protected] -- Speaking for myself only.

The changes for Xeon 55xx are minimal. I did some tests with the driver on it already.

I haven't the datasheets for 75xx, so I can't say for sure if it would be better to use the same driver or to fork it.

Well, the error parsing can be done in kernel space in a standard way provided by the edac interface.

I don't see why not the mcelog userspace shouldn't use the EDAC interface as one of its source, getting memory errors from it, avoiding the need of re-parsing the errors.

Cheers,
Mauro.

You can't use the same driver.

The errors are just numbers which are printed. If you mean with "parsing" splitting up the bitfields that's not really an too interesting case.

Essentially to get terminology clear, for corrected errors there are multiple
steps: (uncorrected errors are quite different)

1) Getting the error from hardware registers
2) Accounting them
3) Presenting them to users
4) Reacting to events
which can be separated in
4a) protocol to communicate with event handler
4a1) interface to wake up event handler
4a2) communication
4b) event handler itself

Some parts of these need to be in kernel space: but that's pretty much only (1)

Some parts of these need to be in user space: in particular 4b) and (3) for any non trivial presentation (the kernel can do some very limited one, but it's not good at anything non trivial here)

4b needs to be in user space, it's deep policies and most interesting advanced reactions to errors cannot be done in kernel space alone.

i7core does (1), some of (2) but not complete and 4a)
I don't really count EDAC as (3) because fishing the numbers out of sysfs by hand is not user friendly. In EDAC that's typically done with the EDAC utils, which are user space.

EDAC doesn't really solve 4a1) unless you could "written a syslog scanner" in it.

The xeon75xx mce driver only does (1) and uses the standard MCE event passing mechanism (4a) to pass it to mcelog.

mcelog just does the other parts, most of which have to be in user space anyways.

The only thing you could probably argue is if it should do accounting or not. Right now it does it and EDAC does it too. At least for advanced accounting (per page) where you can have a lot of data (can be larger than a struct page per 4K page)
I personally prefer that to be swappable.

Hope this helps,

-Andi

[email protected] -- Speaking for myself only.

Does this patch fix it? (I assume your config is missing CONFIG_PCI)

-Andi

---

commit 748e9c041da32f173f20289c71ef0462b8e97234

Author: Andi Kleen <[email protected]>
Date:   Sat Jan 23 12:34:10 2010 +0100

Make xeon75xx memory driver dependent on PCI

Found by Ingo Molnar's automated tester

index f62db24..ab2d0a8 100644
@@ -834,7 +834,7 @@ config X86_MCE_INTEL
 
 config X86_MCE_XEON75XX
 	tristate "Intel Xeon 7500 series corrected memory error driver"
-	depends on X86_MCE_INTEL
+	depends on X86_MCE_INTEL && PCI
 	---help---
 	   Add support for a Intel Xeon 7500 series specific memory error driver.
 	   This allows to report the DIMM and physical address on a corrected

Commit-ID:  757fd770c649b0dfa6eeefc2d5e2ea3119b6be9c
Gitweb:     http://git.kernel.org/tip/757fd770c649b0dfa6eeefc2d5e2ea3119b6be9c
Author:     Andi Kleen <[email protected]>
AuthorDate: Sat, 23 Jan 2010 12:33:59 +0100
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Fri, 5 Feb 2010 14:51:53 -0800

x86, mce: Make xeon75xx memory driver dependent on PCI

Found by Ingo Molnar's automated tester.

LKML-Reference: <[email protected]>

---
 arch/x86/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index f62db24..ab2d0a8 100644
@@ -834,7 +834,7 @@ config X86_MCE_INTEL
 
 config X86_MCE_XEON75XX
 	tristate "Intel Xeon 7500 series corrected memory error driver"
-	depends on X86_MCE_INTEL
+	depends on X86_MCE_INTEL && PCI
 	---help---
 	   Add support for a Intel Xeon 7500 series specific memory error driver.
 	   This allows to report the DIMM and physical address on a corrected

As an x86 maintainer i'm NAK-ing your Nehalem MCE patches. I really dont want to see this code in v2.6.34, please work on it some more - this should be implemented _properly_ and _cleanly_.

Andi, you've ignored my repeated complaints about the cleanliness of the MCE code:

http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-01/msg08454.html

( There's been earlier remarks from me on this topic, months ago, the first
one more than a year ago, so it's not like you didnt have any advance warning. I suspect i should have NAK-ed earlier. )

and you've ignored what the EDAC developers such as Mauro tried to explain to you in that same thread. Your design to expose Intel hardware features sucks (because it's essentially non-existent, and because it exposes so little and gives users so little benefits) and you should know it.

Please work with Mauro on the Nehalem EDAC bits, they seem rather advanced to me for v2.6.34, and _far_ cleaner and more capable as well. See those Intel support bits at:

git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/i7core.git master

  Documentation/edac.txt              |  151 +++
  arch/x86/include/asm/pci_x86.h      |    2 +
  arch/x86/kernel/cpu/mcheck/mce_64.c | 1200 +++++++++++++++++++++
  arch/x86/pci/legacy.c               |   43 +-
  drivers/edac/Kconfig                |   19 +
  drivers/edac/Makefile               |    7 +
  drivers/edac/edac_core.h            |    5 +
  drivers/edac/edac_mc_sysfs.c        |    4 +
  drivers/edac/edac_mce.c             |   61 ++
  drivers/edac/i7core_edac.c          | 1966 +++++++++++++++++++++++++++++++++++
  include/linux/edac_mce.h            |   31 +
  include/linux/pci.h                 |    1 +
  include/linux/pci_ids.h             |   19 +
  13 files changed, 3492 insertions(+), 17 deletions(-)

It's a big step forward for Intel CPU features in my opinion, as it is a proper, clean driver interface, and i find it highly curious why you are not coding for that feature instead.

Once that is done the EDAC code can be pushed by all distros as a common interface, as recent Intel CPUs would be supported by it.

I really do not understand why you are trying to keep Intel CPUs in the dark ages while most other CPUs are properly supported by the EDAC code ... It seems hugely counter-intuitive to me.

Note, any missing functionality on the EDAC side needs a proper driver interface to arch/x86 - not this kind of ugly butchered-in 700 lines piece of ugly crap that you are trying to push here ... I'd welcome patches for such interfaces as they'd further simplify (and also enhance) arch/x86.

( And as i suggested you might also want to work on representing machine
events as part of the perf events framework. )

Ingo

Hi Ingo,

core_i7 and EDAC has nothing to do with this code and it has nothing to do with the problem this patch is solving.

This is for a different chip (xeon75xx)
which has a completely different memory subsystem and reports memory errors in a completely different way than xeon75xx/core_i7.

For core_i7/xeon55xx there is no additional event interface needed; it's all supplied by the hardware on the existing interfaces.

The point of this code is to annotate the CE events on Xeon 75xx and to implement specific backend actions (page offlining, triggers) based on specific events. These backend actions are already implemented on 55xx without additional changes (no need for EDAC)

EDAC does not provide an event interface that can be polled, just counts, so this cannot be done with EDAC. It's simply a topology enumeration with error counts. mcelog is not a topology interface, it's a event notification mechanism.

EDAC and mcelog are orthogonal, they don't solve the same problem.

So your nack is based on incorrect assumptions and doesn't make sense. What you're asking for cannot be done with current EDAC as far as I know.

-Andi

It does not matter at all that current EDAC cannot do that right now. Fact is that you are stubbornly ignoring any request from the x86 maintainers to rework MCE, consolidate it with EDAC and integrate it into perf as the suitable event logging mechanism.

MCE has no design at all, it's a specialized hack which is limited to a specific subset of the overall machine health monitoring and reporting facilities.

You refuse to even think about consolidating the handling of all health monitoring and reporting facilities into a well designed and integrated framework.

Your sole argument is that mce can do it and EDAC or whatever can not. That's not a technical argument at all. MCE does not become a better design just because you hacked another feature into it.

Ingo's NAK is completely correct and he has my full support for it.

We do not want new crap in the already horrible MCE code. We simply request a consolidation of machine health monitoring/reporting facilities before adding new stuff.

You have been ignoring our technical requests for more than a year. You are refusing to work with other people on a well designed solution. You just follow your own agenda and try to squeeze more stuff into MCE.

tglx

Hi Thomas,

I would appreciate if you could read the whole email
and ideally the references too before replying. I apologize for the length, but this is a complicated topic.

The main reason I didn't react to that proposal is I don't see a clear path to make perf a good error mechanism.

I know there's a tendency that if you're working on something that you think is cool, to try to force everything else you're seeing into that model too (I occasionally have such tendencies too :-)

But if you take a step back and look at the requirements with a sceptical eye that's not always the best thing to do.

Requirements for error handling are very different from performance monitoring.

Let me walk you through some of these differences:

USER TOOLS:

The current perf user tools are not suitable for errors: they are not "always on running in the background" like you need for errors.

They are aimed at a interactive user model which is fine for performance monitoring (well at least some forms of performance monitoring), but not for errors.

Yes they could be probably reworked for a "always on" daemon model, but the result would be

a) completely different than what you have today in terms of interface (it would be a lot more like you have with oprofile, and as I understand one of the main motivations for perf was wide spread dislike of the oprofile daemon model)
b) likely worse for performance monitoring (unless you fork them into two) The requirements are simply very different.
c) a lot like what mcelog is today. mcelog today is a always on error daemon optimized for error handling, nothing else.

There's no associated error oriented infrastructure like triggers etc. in perf Yes that could be all implemented, but (b) and (c) above apply.

So yes it could be probably done, but I suspect the result would not make you happy for performance monitoring.

EVENT INTERFACE I:

The perf interface is aimed at a specific way of filtering events, which is not the right interface for errors, because you need usually all errors in most (not all) cases. Basically in performance monitoring typically most events are off and you sometimes turn them on, in error handling it's exactly the other way around.

Also errors tend to have different behavior from performance counters, for example a model for a error on a object
is more the "leaky bucket", which is not a good fit for performance.

(I have more details on this in http://halobates.de/plumbers-error.pdf)

OVERHEAD:

The perf subsystem has relatively high overhead today (in terms of memory size and code size overhead) and is IMHO not suitable to be always active because of this.

Errors are very fundamental and error reporting mechanisms have to be always active, so it's extremely important that they have very low overhead by default. That's not what perf's model is: it trades memory size and code size for more performance. That is fine for optional monitoring (if you can afford it), but not the right model for an fundamental "always on" mechanism. For "always on" infrastructure it's better to be slim.

That said I suspect perf events could be likely put on a serious diet, but it's unclear if the result would work as well as it does today
for performance monitoring. You would likely lose some features optimized for it at least.

EVENT INTERFACE II:

Partly that's because it has a lot of functionality that are not needed for errors anyways. For example error just needs some very simple error buffers that can be straight forwardly implemented using kfifos (I did that already in fact). That's just a few lines, all the functionality in kernel/perf/* is not really needed.

There's no good way to throttle events per source, like it's needed for errors.

EVENT INTERFACE III:

Then one of the current issues with mcelog is that it's not straight forward to add variable length record types with typing etc. This isn't too big a problem for MCEs (although the DIMM error reporting would have been slightly nicer with it) but for some other types of errors it's a bigger issue.

Now the funny thing is (and I keep waiting for Ingo to figure that out :-):
the perf record format has exactly the same problem as mcelog in this regard. It's a untyped binary format where it's only possible to add something at the end, not a fully extensible typed format with sub records etc.

A better match would be either netlink with its sub record (although for various reasons other I don't think it's the best model either) or the ASCII based udev sysfs interfaces.

In fact that is what Ingo asked for some time ago (before he
moved to the "everything must be perf" model). He wanted an ASCII interface (so more like the udev model). I'm not completely happy with that either, but it's probably still one of the better models and could be made to work.

It's definitely not perf though.

First I work with a lot of people on error handling, even if you're not always in Cc.

We would need to agree to disagree on EDAC being a "well designed solution) IMHO it has a lot of problems (not just in my opinion if you read some of the mails e.g. from Borislav he's stating the same) and it's definitely not the general frame work you're asking for In fact in many ways EDAC far more specialized to some specific subset of errors than mcelog.

A generic error frame work (that would be neither EDAC nor perf nor mcelog on the interface level) could be probably done and I have some ideas on how to do that properly (e.g. see the link below), but it's not a short term project. It needs a lot of design work to be done properly and also would likely need to evolve for some time. It would also need a suitable user level infrastructure, which is actually a larger project than the kernel interfaces.

The patch above was simply intended to solve a specific problem on a specific chip. I don't claim the interface is the best I ever did (definitely not), but at least it solves an existing problem in a relatively straight forward way and I claim there's no clear better solution with today's infrastructure.

How are you suggesting to solve the DIMM error reporting in the short term (let's say 2.6.34/35 time frame, without major redesigns) ?

-Andi

References:
- Thoughts on future error handling model:
http://halobates.de/plumbers-error.pdf
- mcelog kernel and userland design today:
http://halobates.de/mce-lc09-2.pdf

[email protected] -- Speaking for myself only.

On Fri, Feb 19, 2010 at 01:17:34PM +0100, Andi Kleen wrote:

<snip all the perf analysis>

I think you're missing the point - it doesn't have to be perf. It could just as well be some other tool which _shares_ functionality with perf. See this last mail from Ingo:
http://marc.info/?l=linux-kernel&m=126635420607236 on which you were also CCed, by the way, where he suggests that we could have a tool called 'hw' which reuses functionality with perf but concentrates on error handling and all that RAS functionality in modern CPUs. It should also have a daemon component etc...

Sorry, but from our last discussion on attempting to work towards such an infrastructure solution I got the same impression as Thomas and Ingo that you're simply not willing to work together on getting a real thing done. That's why I stopped bothering - it simply made no sense to me to waste time in fruitless discussions.

Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

Borislav,

That was one of my points, but there were others too about the suitability of the kernel infra structure and the interfaces.

I don't think the perf syscall interface (nor the internal implement) in its current form is good for errors and also see no clear path to make it so (without basically adding another different interface)

So you would have different interfaces: you don't really need new syscalls for this (read/write is fine) and perf_counter_open() in its current form is completely unsuitable for errors (unless your error reporting registers look like a x86 PMU -- mine doesn't at least) And different userland. And the kernel internal requirements are very different too (see previous email)

Where is the commonality with perf?

On the tool side:

I'm working on such a tool already for quite some time. It's called mcelog. Now it uses an older interface today, but at some point I would expect it to move to other interfaces too (e.g. next step for that would be APEI errors)

If you only knew mcelog from a few years ago: it's quite different today than it was and please look again.

That is the end result will be likely called different (it doesn't make much sense to call something that handles all kinds of errors "mcelog") and also some stuff needs to be more generic, but I suspect it'll share quite some concepts.

If the only problem is the naming we can probably work something out? In principle it could be called "hw", but the name seems awfully generic, especially for a daemon. I was more tending something like "errord" or so.

On the topology: I was not trying to replace existing topology tools (like lscpu, lspci etc.). I don't see any major problems (apart from some details that don't deserve a redesign) with them.

Well I keep ignoring suggestions to put more stuff into EDAC, mostly because I think the EDAC design needs to be thrown out instead of extended. Are you referring to that?

My impression was that you got to the same conclusion (at least for parts of current EDAC like the events) based on your earlier emails.

The current issue is less in enumeration/topology anyways but more in event handling I would say. In the end topology/enumeration is the easier part, and most of it is already working quite well.

I'm trying to do things step by step, including for short term problems extending current interfaces if possible and then longer term moving to new better interfaces.

-Andi

Andi,

Ingo and Thomas are right: we need to work together to improve the error report, and this means that we shouldn't ignore putting more stuff in EDAC.

EDAC is generic enough to work with different type of memory and memory controllers, and to provide a consistent interface to describe it on a way that userspace doesn't need to know what are the error registers at the hardware, nor how to decode a "magic" error number into something that has a meaning.

As Boris properly pointed, EDAC has space for improvements, and part of the perf logic can be used as a start point to give some flash new ideas.

The main issue I see with MCE is at the interface level. I think if we all cope together, we can converge into a proper interface that will be accepted upstream.

Cheers,
Mauro

Well the main problem I have with EDAC is that it has far too much information (e.g. down to ranks/banks and also too much information on
the internal topology of the memory controller, and it can't even express some current designs).

For me it looks like it was designed by someone starring at a motherboard/DIMM semantics plan, and I don't think that's the right level to think about these things.

Going that deep typically
requires very hardware specific information and in some cases it's not even possible. I also don't think it's useful information to present (and it's really the opposite of "abstraction")

I also have yet to see a useful use case where you need to look "inside" a DIMM on the reporting level. The useful level is typically the "FRU" (something you can replace), with only some very specific extensions for special use cases.

There's also no generic way to do the necessary enumeration down to the level EDAC needs. For some cases hardware specific drivers can be written, but it's always better if the generic case works in a architectural way.

Then it does all the enumeration on the kernel, but there
are no useful facilities to sync that with a user level representation. And most of the useful advannced & interesting RAS features I'm interested in need user level support.

I prefer at least for MCE to stay on the architectural level with only minor extensions for specific use cases.

Now to address these problems you could throw large parts of EDAC out (do you mean that with 'flexible enough'?) and then add a actual event interface (working on the later is my plan)

See my analysis several mails up. Which parts of perf do you want to actually use? I don't see any that's actually directly usable without major changes.

Just that we're on the same level, could you spell out in detail what problems you're seeing with it?

[I'm not claiming there are none, I'm just curious what you think they are]

-Andi

I was thinking on a way where we could work with the EDAC/MCE issues, and a way for us to move ahead. My proposal is to organize an EDAC/MCE BoF session or a mini-summit during the Collaboration Summit, in San Francisco, with the interested parties. I suspect that some of us will be there already.

It shouldn't be hard to find some place there for us to take a look at the EDAC architecture and come with some proposal.

As today is the last day for CFP, I've also submitted there a proposal for a panel. If approved, we can use it to collect data from hardware error users (sysops and other users that require high availability on their services), for us to discuss some strategies to address the issue or to summarize what will be discussed on the event.

Comments?

Cheers,
Mauro

On Fri, Feb 19, 2010 at 10:14:17PM -0200, Mauro Carvalho Chehab wrote:

Mauro,

I didn't plan to be there so far. A BoF is probably a good idea, and also looking closely at EDAC together,
but it would be better at some more kernel focussed conference.

If everyone else is at that summit I can try to come, but it would be likely difficult.

We could probably do some kind of online BoF shorter time (e.g. using some chat setup or on the phone)

Proposal for what exactly?

Is this for a event interface or for a topology interface or both or something else entirely?

My personal plan so far was to work on the APEI interface and then possibly look at migrating MCE to that infrastructure too, while updating mcelog to talk to it. This would be mostly addressing events so far.

You want to collect error rates or want to collect use cases?

For a serious collection doing it online would probably give better coverage.

I do both to some degree already.

-Andi

[email protected] -- Speaking for myself only.

Andi,

Thanks for confirming the main problem here:

_You_ do not react, because _you_ do not see a clear path.

So _you_ ignore any request for a major cleanup of that whole MCE mess and just keep on tinkering with the existing crap.

And you really expect us to tolerate your ignorance forever and just apply happily more crap to something which should have never been merged in the first place.

<snip>

The patch does not solve a specific problem. It merily adds extra info retrieved from this specific chip. This is simply a new feature which you hacked into the existing mce code.

There will always be a next generation chip with a specific new feature and it does _NOT_ matter at all whether support for such a new feature is straight forward in your opinion or not. Straight forward crap is still crap.

The whole point is that we are not longer accepting whacky addons to MCE w/o seing any collaborative reaction from your side on our technical requests to fix up the whole thing for real.

You can point me to as many PDFs as you want. I'm not interested in stuff you are breeding on your own w/o talking to EDAC folks and trying to address the concerns which were raised by Ingo, myself and others.

Thanks,

tglx

If my understanding is correct, your patch doesn't interact with xeon75xx processor itself, but with the associated chip (I/O hub, aka Boxboro, you abbreviated it to BXB) at least at first of all.

Then since MCE codes contain rather "generic, processor oriented" stuffs while EDAC codes contain relatively "specific, chipset oriented" stuffs, people can suppose that this "arch/x86/kernel/cpu/mcheck/mce-xeon75xx.c" could be implemented in different way and the file could have a different name like "drivers/edac/i7500_edac.c" or so.

However the real issue is that I couldn't figure out why this code requires new hook in the polling handler, what is PFA, and what kind of restriction let you to do so. You noted that "The act of retrieving the DIMM/PA information can take some time" but I'm not sure it is safe even if poll handler is called via CMCI.

Anyway, Andi, could you point proper specification or datasheet to know/check what you are going to do here?
Otherwise I could not distinguish your work from black magic...

Thanks,
H.Seto