>> All of this stuff only applies to server systems - so quibbling over
>> a handful of *bytes* in an error record on a system that has tens,
>> hundreds or even thousands of *gigabytes* of memory seems
>> a bit pointless.
>
> But there's still only a limited number of bytes in the ring buffer no
> matter what the system, thus we still need to quibble over it.
To which I'll counter that the trace ring buffer can handle tracing of
events like page faults and context switches (can't it?) that happen
at a rate of thousands per second. Our eMCA records will normally
happen at a rate of X per month (where X may well be less than one).
If there is a storm of errors - we disable CMCI interrupts and revert
to polling. We declare a "storm" as just 15 events in a second. If we
switch to polling, then we won't poll faster than once per second.
So worst case is that we are seeing some steady flow of events that
don't quite trigger the storm detector ... about 14 events per second.
-Tony
On Mon, 2 Jun 2014 16:22:19 +0000
"Luck, Tony" <[email protected]> wrote:
> To which I'll counter that the trace ring buffer can handle tracing of
> events like page faults and context switches (can't it?) that happen
> at a rate of thousands per second. Our eMCA records will normally
> happen at a rate of X per month (where X may well be less than one).
> If there is a storm of errors - we disable CMCI interrupts and revert
> to polling. We declare a "storm" as just 15 events in a second. If we
> switch to polling, then we won't poll faster than once per second.
>
> So worst case is that we are seeing some steady flow of events that
> don't quite trigger the storm detector ... about 14 events per second.
Also matters how big you expect these events to be. If you get a
"christmas tree" set of flags, how big will that event grow with all
the descriptions attached?
The max event size after all headers is 4056 bytes. If you go over
that, the event is ignored.
-- Steve
On Mon, Jun 02, 2014 at 12:57:48PM -0400, Steven Rostedt wrote:
> Also matters how big you expect these events to be. If you get a
> "christmas tree" set of flags, how big will that event grow with all
> the descriptions attached?
>
> The max event size after all headers is 4056 bytes. If you go over
> that, the event is ignored.
>
Hi, Steven
Normally, the length of one eMCA trace record is between 200 and 256 bytes.
Once CMCI storm happens, before it is turned into poll mode, there are
about ~15 CMCI events are recorded, because I don't use rate limit for
trace so they should be recorded so seriously, some records will be lost.
But they are repeated and similar records so maybe the *lost* is not a
big issue.
Return to how to print trace record. To avoid buffer waste, I need to
print data when TP_printk called, in the meanwhile, the print content
is an array of [name, value], but we don't know how many items are
valid. Here is the question: I can't create a dynamic printk format
like "%s %d, %s %d, ..." in TP_printk. So the only way to me is
printking them all, even some of them are invalid, which means an 12
group "%s %d", or somthing like "%.*s" to make output format graceful.
This is what we want?
On Tue, 3 Jun 2014 04:36:07 -0400
"Chen, Gong" <[email protected]> wrote:
> On Mon, Jun 02, 2014 at 12:57:48PM -0400, Steven Rostedt wrote:
> > Also matters how big you expect these events to be. If you get a
> > "christmas tree" set of flags, how big will that event grow with all
> > the descriptions attached?
> >
> > The max event size after all headers is 4056 bytes. If you go over
> > that, the event is ignored.
> >
> Hi, Steven
>
> Normally, the length of one eMCA trace record is between 200 and 256 bytes.
> Once CMCI storm happens, before it is turned into poll mode, there are
> about ~15 CMCI events are recorded, because I don't use rate limit for
> trace so they should be recorded so seriously, some records will be lost.
> But they are repeated and similar records so maybe the *lost* is not a
> big issue.
>
> Return to how to print trace record. To avoid buffer waste, I need to
> print data when TP_printk called, in the meanwhile, the print content
> is an array of [name, value], but we don't know how many items are
> valid. Here is the question: I can't create a dynamic printk format
> like "%s %d, %s %d, ..." in TP_printk. So the only way to me is
> printking them all, even some of them are invalid, which means an 12
> group "%s %d", or somthing like "%.*s" to make output format graceful.
> This is what we want?
You can create a helper function to call (needs to be placed in a .c
file).
Note, there's a pointer to a trace_seq structure "p" that is available.
Hmm, I should add a get_dynamic_array_len(field), to give you the
length. I'll add that now. I also don't like the trace_seq being "p" as
that is too generic. Maybe I'll change that to "__trace_seq" or
something not so generic.
Anyway, have something like this:
TP_printk("%s", emca_parse_events(p, __get_dynamic_array(field),
__get_dynamic_array_len(field)));
I'll still need to add that __get_dynamic_array_len() helper. I'll send
you something tonight.
Then you write the emca_parse_events() as:
const char *emca_parse_events(struct trace_seq *p,
struct cper_sec_mem_rec *data, int len)
{
const char *ret = p->buffer + p->len;
int i;
len = len / sizeof(struct cper_sec_mem_rec);
for (i = 0; i < len; i++) {
switch (data[i].type) {
case FOO:
trace_seq_printf(p, "BAR: %d\n", data[i].data);
break;
[..]
}
}
trace_seq_putc('\0');
return ret;
}
-- Steve
On Tue, 3 Jun 2014 10:35:44 -0400
Steven Rostedt <[email protected]> wrote:
> I'll still need to add that __get_dynamic_array_len() helper. I'll send
> you something tonight.
>
I got caught up in other work, but I wrote it this morning and I'm
adding it to my 3.16 queue. Thus, you can use this:
-- Steve
>From beba4bb096201ceec0e8cfb7ce3172a53015bdaf Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <[email protected]>
Date: Wed, 4 Jun 2014 14:29:33 -0400
Subject: [PATCH] tracing: Add __get_dynamic_array_len() macro for trace events
If a trace event uses a dynamic array for something other than a string
then there's currently no way the TP_printk() can figure out what size
it is. A __get_dynamic_array_len() is required to know the length.
This also simplifies the __get_bitmask() macro which required it as well,
but instead just hardcoded it.
Signed-off-by: Steven Rostedt <[email protected]>
---
include/trace/ftrace.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 9b7a989..0fd06fe 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -203,6 +203,10 @@
#define __get_dynamic_array(field) \
((void *)__entry + (__entry->__data_loc_##field & 0xffff))
+#undef __get_dynamic_array_len
+#define __get_dynamic_array_len(field) \
+ ((__entry->__data_loc_##field >> 16) & 0xffff)
+
#undef __get_str
#define __get_str(field) (char *)__get_dynamic_array(field)
@@ -211,7 +215,7 @@
({ \
void *__bitmask = __get_dynamic_array(field); \
unsigned int __bitmask_size; \
- __bitmask_size = (__entry->__data_loc_##field >> 16) & 0xffff; \
+ __bitmask_size = __get_dynamic_array_len(field); \
ftrace_print_bitmask_seq(p, __bitmask, __bitmask_size); \
})
@@ -636,6 +640,7 @@ static inline void ftrace_test_probe_##call(void) \
#undef __print_symbolic
#undef __print_hex
#undef __get_dynamic_array
+#undef __get_dynamic_array_len
#undef __get_str
#undef __get_bitmask
@@ -700,6 +705,10 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
#define __get_dynamic_array(field) \
((void *)__entry + (__entry->__data_loc_##field & 0xffff))
+#undef __get_dynamic_array_len
+#define __get_dynamic_array_len(field) \
+ ((__entry->__data_loc_##field >> 16) & 0xffff)
+
#undef __get_str
#define __get_str(field) (char *)__get_dynamic_array(field)
--
1.8.1.4
On Tue, Jun 03, 2014 at 10:35:44AM -0400, Steven Rostedt wrote:
> Note, there's a pointer to a trace_seq structure "p" that is available.
> Hmm, I should add a get_dynamic_array_len(field), to give you the
> length. I'll add that now. I also don't like the trace_seq being "p" as
> that is too generic. Maybe I'll change that to "__trace_seq" or
> something not so generic.
>
>
> Anyway, have something like this:
>
>
> TP_printk("%s", emca_parse_events(p, __get_dynamic_array(field),
> __get_dynamic_array_len(field)));
>
Hi, Steven & other guys
Here is the new patch for eMCA trace. I post it here as a RFC patch for
discussion. Once it is OK, I will resend a new complete patch series.
Before showing the patch I have something to highlight first:
1. I don't use key/value because it is not most efficient way because
it often has 9+ valid fields in one record, which means at least 9 bytes
for type(assuming packing type via __ffs). But original filed, which
is a bit field combination (an u64 field) is enough to cover everyting
without *ffs* expense. As for those fields with different lenght, it is
a disater to save them in minimum cost.
2. To share more codes like decoding error. If I use key/value I must
rewrite codes & add more obsure enum, definitions etc. Currently I just
adopt a lightweight compact struct to save ring buffer spaces. Please
refer to the codes.
3. I use same buffer to store decoded string, for dimm location, memory
location, even intermediate trace result because in our current desgin,
trace output and demsg output are mutually exclusive, so don't worry
about potential contention.
One more thing, all I saved is just when one reads raw data from user space,
TP_printk part can be bypassed so extra storage expense is none.
Am I right, Steven?
Here is the dmesg log:
[ 267.627996] {1}Hardware error detected on CPU15
[ 267.628007] {1}It has been corrected by h/w and requires no further action
[ 267.628011] {1}event severity: corrected
[ 267.628017] {1} Error 0, type: corrected
[ 267.628022] {1} section_type: memory error
[ 267.628026] {1} physical_address: 0x000000084f5d8000
[ 267.628037] {1} node: 2 card: 0 module: 0 rank: 0 bank: 0 row: 28717 column: 1528
[ 267.628057] {1} DIMM location: Memriser3 CHANNEL A DIMM 0
[ 316.135078] {2}Hardware error detected on CPU15
[ 316.135095] {2}It has been corrected by h/w and requires no further action
[ 316.135102] {2}event severity: corrected
[ 316.135107] {2} Error 0, type: corrected
[ 316.135111] {2} section_type: memory error
[ 316.135114] {2} physical_address: 0x000000084f6c0000
[ 316.135120] {2} node: 2 card: 0 module: 0 rank: 0 bank: 0 row: 28732 column: 1504
[ 316.135124] {2} DIMM location: Memriser3 CHANNEL A DIMM 0
Here is the trace log (when dmesg is disabled):
<idle>-0 [015] d.h3 207.156998: extlog_mem_event: {1} corrected error: unknown physical addr: 00000008527e2000 (mask lsb: ff) node: 2 card: 0 module: 0 rank: 0 bank: 0 row: 29758 column: 1616 DIMM location: Memriser3 CHANNEL A DIMM 0 FRU: 00000000-0000-0000-0000-000000000000
<idle>-0 [015] d.h3 339.574445: extlog_mem_event: {2} corrected error: unknown physical addr: 0000000851dba000 (mask lsb: ff) node: 2 card: 0 module: 0 rank: 0 bank: 0 row: 29803 column: 1592 DIMM location: Memriser3 CHANNEL A DIMM 0 FRU: 00000000-0000-0000-0000-000000000000
--------8<--------
From 864bd0f2d2121b8c7a941e6e776952378a6c636c Mon Sep 17 00:00:00 2001
From: "Chen, Gong" <[email protected]>
Date: Fri, 6 Jun 2014 01:46:45 -0400
Subject: [PATCH 5/7] trace, RAS: Add eMCA trace event interface
Add trace interface to elaborate all H/W error related information.
v7 -> v6: compact trace info to save some trace buffer space
v6 -> v5: format adjustment.
v5 -> v4: Add physical mask(LSB) in trace.
v4 -> v3: change ras trace dependency rule.
v3 -> v2: minor adjustment according to the suggestion from Boris.
v2 -> v1: spinlock is not needed anymore.
Signed-off-by: Chen, Gong <[email protected]>
---
drivers/acpi/Kconfig | 4 ++-
drivers/acpi/acpi_extlog.c | 27 +++++++++++++++++---
drivers/firmware/efi/cper.c | 48 ++++++++++++++++++++++++++++++++---
drivers/ras/ras.c | 1 +
include/linux/cper.h | 21 +++++++++++++++
include/ras/ras_event.h | 62 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 155 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a34a228..099a2d5 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -370,6 +370,7 @@ config ACPI_EXTLOG
tristate "Extended Error Log support"
depends on X86_MCE && X86_LOCAL_APIC
select UEFI_CPER
+ select RAS_TRACE
default n
help
Certain usages such as Predictive Failure Analysis (PFA) require
@@ -384,6 +385,7 @@ config ACPI_EXTLOG
Enhanced MCA Logging allows firmware to provide additional error
information to system software, synchronous with MCE or CMCI. This
- driver adds support for that functionality.
+ driver adds support for that functionality with corresponding
+ tracepoint which carries that information to userspace.
endif # ACPI
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index c4a5d87..3c4a8aa 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -16,6 +16,7 @@
#include <asm/mce.h>
#include "apei/apei-internal.h"
+#include <ras/ras_event.h>
#define EXT_ELOG_ENTRY_MASK GENMASK_ULL(51, 0) /* elog entry address mask */
@@ -137,8 +138,12 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
struct mce *mce = (struct mce *)data;
int bank = mce->bank;
int cpu = mce->extcpu;
- struct acpi_generic_status *estatus;
- int rc;
+ struct acpi_generic_status *estatus, *tmp;
+ struct acpi_generic_data *gdata;
+ const uuid_le *fru_id = &NULL_UUID_LE;
+ char *fru_text = "";
+ uuid_le *sec_type;
+ static u32 err_seq;
estatus = extlog_elog_entry_check(cpu, bank);
if (estatus == NULL)
@@ -148,7 +153,23 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
/* clear record status to enable BIOS to update it again */
estatus->block_status = 0;
- rc = print_extlog_rcd(NULL, (struct acpi_generic_status *)elog_buf, cpu);
+ tmp = (struct acpi_generic_status *)elog_buf;
+ print_extlog_rcd(NULL, tmp, cpu);
+
+ /* log event via trace */
+ err_seq++;
+ gdata = (struct acpi_generic_data *)(tmp + 1);
+ if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
+ fru_id = (uuid_le *)gdata->fru_id;
+ if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
+ fru_text = gdata->fru_text;
+ sec_type = (uuid_le *)gdata->section_type;
+ if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
+ struct cper_sec_mem_err *mem = (void *)(gdata + 1);
+ if (gdata->error_data_length >= sizeof(*mem))
+ trace_extlog_mem_event(mem, err_seq, fru_id, fru_text,
+ (u8)gdata->error_severity);
+ }
return NOTIFY_STOP;
}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 64d182f..23d0cef 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -207,7 +207,7 @@ const char *cper_mem_err_type_str(unsigned int etype)
}
EXPORT_SYMBOL_GPL(cper_mem_err_type_str);
-int cper_mem_err_location(const struct cper_sec_mem_err *mem, char *msg)
+int cper_mem_err_location(struct cper_mem_err_compact *mem, char *msg)
{
u32 len, n;
@@ -249,7 +249,7 @@ int cper_mem_err_location(const struct cper_sec_mem_err *mem, char *msg)
return n;
}
-int cper_dimm_err_location(const struct cper_sec_mem_err *mem, char *msg)
+int cper_dimm_err_location(struct cper_mem_err_compact *mem, char *msg)
{
u32 len, n;
const char *bank = NULL, *device = NULL;
@@ -271,8 +271,47 @@ int cper_dimm_err_location(const struct cper_sec_mem_err *mem, char *msg)
return n;
}
+void cper_mem_err_pack(const struct cper_sec_mem_err *mem, void *data)
+{
+ struct cper_mem_err_compact *cmem = (struct cper_mem_err_compact *)data;
+
+ cmem->validation_bits = mem->validation_bits;
+ cmem->node = mem->node;
+ cmem->card = mem->card;
+ cmem->module = mem->module;
+ cmem->bank = mem->bank;
+ cmem->device = mem->device;
+ cmem->row = mem->row;
+ cmem->column = mem->column;
+ cmem->bit_pos = mem->bit_pos;
+ cmem->requestor_id = mem->requestor_id;
+ cmem->responder_id = mem->responder_id;
+ cmem->target_id = mem->target_id;
+ cmem->rank = mem->rank;
+ cmem->mem_array_handle = mem->mem_array_handle;
+ cmem->mem_dev_handle = mem->mem_dev_handle;
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_pack);
+
+const char *cper_mem_err_unpack(struct trace_seq *p, void *data)
+{
+ struct cper_mem_err_compact *cmem = (struct cper_mem_err_compact *)data;
+ const char *ret = p->buffer + p->len;
+
+ if (cper_mem_err_location(cmem, rcd_decode_str))
+ trace_seq_printf(p, "%s", rcd_decode_str);
+ if (cper_dimm_err_location(cmem, rcd_decode_str))
+ trace_seq_printf(p, "%s", rcd_decode_str);
+ trace_seq_putc(p, '\0');
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
+
static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
{
+ struct cper_mem_err_compact cmem;
+
if (mem->validation_bits & CPER_MEM_VALID_ERROR_STATUS)
printk("%s""error_status: 0x%016llx\n", pfx, mem->error_status);
if (mem->validation_bits & CPER_MEM_VALID_PA)
@@ -281,14 +320,15 @@ static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem)
if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
printk("%s""physical_address_mask: 0x%016llx\n",
pfx, mem->physical_addr_mask);
- if (cper_mem_err_location(mem, rcd_decode_str))
+ cper_mem_err_pack(mem, &cmem);
+ if (cper_mem_err_location(&cmem, rcd_decode_str))
printk("%s%s\n", pfx, rcd_decode_str);
if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE) {
u8 etype = mem->error_type;
printk("%s""error_type: %d, %s\n", pfx, etype,
cper_mem_err_type_str(etype));
}
- if (cper_dimm_err_location(mem, rcd_decode_str))
+ if (cper_dimm_err_location(&cmem, rcd_decode_str))
printk("%s%s\n", pfx, rcd_decode_str);
}
diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index 4cac43a..da227a3 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -23,4 +23,5 @@ static int __init ras_init(void)
}
subsys_initcall(ras_init);
+EXPORT_TRACEPOINT_SYMBOL_GPL(extlog_mem_event);
EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index ed088b9..3548160 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -22,6 +22,7 @@
#define LINUX_CPER_H
#include <linux/uuid.h>
+#include <linux/trace_seq.h>
/* CPER record signature and the size */
#define CPER_SIG_RECORD "CPER"
@@ -363,6 +364,24 @@ struct cper_sec_mem_err {
__u16 mem_dev_handle; /* module handle in UEFI 2.4 */
};
+struct cper_mem_err_compact {
+ __u64 validation_bits;
+ __u16 node;
+ __u16 card;
+ __u16 module;
+ __u16 bank;
+ __u16 device;
+ __u16 row;
+ __u16 column;
+ __u16 bit_pos;
+ __u64 requestor_id;
+ __u64 responder_id;
+ __u64 target_id;
+ __u16 rank;
+ __u16 mem_array_handle;
+ __u16 mem_dev_handle;
+};
+
struct cper_sec_pcie {
__u64 validation_bits;
__u32 port_type;
@@ -406,5 +425,7 @@ const char *cper_severity_str(unsigned int);
const char *cper_mem_err_type_str(unsigned int);
void cper_print_bits(const char *prefix, unsigned int bits,
const char * const strs[], unsigned int strs_size);
+void cper_mem_err_pack(const struct cper_sec_mem_err *, void *);
+const char *cper_mem_err_unpack(struct trace_seq *, void *);
#endif
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index acbcbb8..66cd7bb 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -9,6 +9,68 @@
#include <linux/edac.h>
#include <linux/ktime.h>
#include <linux/aer.h>
+#include <linux/cper.h>
+
+/*
+ * MCE Extended Error Log trace event
+ *
+ * These events are generated when hardware detects a corrected or
+ * uncorrected event.
+ */
+
+/* memory trace event */
+
+TRACE_EVENT(extlog_mem_event,
+ TP_PROTO(struct cper_sec_mem_err *mem,
+ u32 err_seq,
+ const uuid_le *fru_id,
+ const char *fru_text,
+ u8 sev),
+
+ TP_ARGS(mem, err_seq, fru_id, fru_text, sev),
+
+ TP_STRUCT__entry(
+ __field(u32, err_seq)
+ __field(u8, etype)
+ __field(u8, sev)
+ __field(u64, pa)
+ __field(u8, pa_mask_lsb)
+ __dynamic_array(char, fru, 48)
+ __dynamic_array(u8, data, sizeof(struct cper_mem_err_compact))
+ ),
+
+ TP_fast_assign(
+ __entry->err_seq = err_seq;
+ if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
+ __entry->etype = mem->error_type;
+ else
+ __entry->etype = ~0;
+ __entry->sev = sev;
+ if (mem->validation_bits & CPER_MEM_VALID_PA)
+ __entry->pa = mem->physical_addr;
+ else
+ __entry->pa = ~0ull;
+
+ if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
+ __entry->pa_mask_lsb =
+ (u8)__ffs64(mem->physical_addr_mask);
+ else
+ __entry->pa_mask_lsb = ~0;
+ snprintf(__get_dynamic_array(fru), 47,
+ "FRU: %pUl %.20s", fru_id, fru_text);
+ cper_mem_err_pack(mem, __get_dynamic_array(data));
+ ),
+
+ TP_printk("{%d} %s error: %s physical addr: %016llx (mask lsb: %x) %s%s",
+ __entry->err_seq,
+ cper_severity_str(__entry->sev),
+ cper_mem_err_type_str(__entry->etype),
+ __entry->pa,
+ __entry->pa_mask_lsb,
+ cper_mem_err_unpack(p, __get_dynamic_array(data)),
+ __get_str(fru))
+);
/*
* Hardware Events Report
--
2.0.0.rc2
On Fri, 6 Jun 2014 02:51:41 -0400
"Chen, Gong" <[email protected]> wrote:
> +/*
> + * MCE Extended Error Log trace event
> + *
> + * These events are generated when hardware detects a corrected or
> + * uncorrected event.
> + */
> +
> +/* memory trace event */
> +
> +TRACE_EVENT(extlog_mem_event,
> + TP_PROTO(struct cper_sec_mem_err *mem,
> + u32 err_seq,
> + const uuid_le *fru_id,
> + const char *fru_text,
> + u8 sev),
> +
> + TP_ARGS(mem, err_seq, fru_id, fru_text, sev),
> +
> + TP_STRUCT__entry(
> + __field(u32, err_seq)
> + __field(u8, etype)
> + __field(u8, sev)
> + __field(u64, pa)
> + __field(u8, pa_mask_lsb)
> + __dynamic_array(char, fru, 48)
> + __dynamic_array(u8, data, sizeof(struct cper_mem_err_compact))
For constant size arrays, don't use __dynamic_array() just use
__array().
Although I'd get rid of the fru and replace that with:
__field(unsigned long, fru_id)
__string(fru_text, fru_text)
> + ),
> +
> + TP_fast_assign(
> + __entry->err_seq = err_seq;
> + if (mem->validation_bits & CPER_MEM_VALID_ERROR_TYPE)
> + __entry->etype = mem->error_type;
> + else
> + __entry->etype = ~0;
> + __entry->sev = sev;
> + if (mem->validation_bits & CPER_MEM_VALID_PA)
> + __entry->pa = mem->physical_addr;
> + else
> + __entry->pa = ~0ull;
> +
> + if (mem->validation_bits & CPER_MEM_VALID_PA_MASK)
> + __entry->pa_mask_lsb =
> + (u8)__ffs64(mem->physical_addr_mask);
> + else
> + __entry->pa_mask_lsb = ~0;
> + snprintf(__get_dynamic_array(fru), 47,
> + "FRU: %pUl %.20s", fru_id, fru_text);
Although, why not just save the id and text straight? Why format it
here?
__entry->fru_id = fru_id;
__assign_str(fru_text, fru_text);
> + cper_mem_err_pack(mem, __get_dynamic_array(data));
> + ),
> +
> + TP_printk("{%d} %s error: %s physical addr: %016llx (mask lsb: %x) %s%s",
TP_printk("{%d} %s error: %s physical addr: %016llx (mask lsb: %x) %s FRU: %pUl %.20s",
> + __entry->err_seq,
> + cper_severity_str(__entry->sev),
> + cper_mem_err_type_str(__entry->etype),
> + __entry->pa,
> + __entry->pa_mask_lsb,
> + cper_mem_err_unpack(p, __get_dynamic_array(data)),
> + __get_str(fru))
__entry->fru_id,
__get_str(fru_text))
-- Steve
> +);
>
> /*
> * Hardware Events Report
On Fri, Jun 06, 2014 at 11:21:27AM -0400, Steven Rostedt wrote:
> Date: Fri, 6 Jun 2014 11:21:27 -0400
> From: Steven Rostedt <[email protected]>
> To: "Chen, Gong" <[email protected]>
> Cc: "Luck, Tony" <[email protected]>, Borislav Petkov <[email protected]>,
> "[email protected]" <[email protected]>,
> "[email protected]" <[email protected]>, LKML
> <[email protected]>
> Subject: Re: [PATCH 5/7 v6] trace, RAS: Add eMCA trace event interface
> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu)
>
> On Fri, 6 Jun 2014 02:51:41 -0400
> "Chen, Gong" <[email protected]> wrote:
>
>
> > +/*
> > + * MCE Extended Error Log trace event
> > + *
> > + * These events are generated when hardware detects a corrected or
> > + * uncorrected event.
> > + */
> > +
> > +/* memory trace event */
> > +
> > +TRACE_EVENT(extlog_mem_event,
> > + TP_PROTO(struct cper_sec_mem_err *mem,
> > + u32 err_seq,
> > + const uuid_le *fru_id,
> > + const char *fru_text,
> > + u8 sev),
> > +
> > + TP_ARGS(mem, err_seq, fru_id, fru_text, sev),
> > +
> > + TP_STRUCT__entry(
> > + __field(u32, err_seq)
> > + __field(u8, etype)
> > + __field(u8, sev)
> > + __field(u64, pa)
> > + __field(u8, pa_mask_lsb)
> > + __dynamic_array(char, fru, 48)
> > + __dynamic_array(u8, data, sizeof(struct cper_mem_err_compact))
>
> For constant size arrays, don't use __dynamic_array() just use
> __array().
>
Thanks a lot! I will update it.
BTW, any comments from other guys? Boris, Tony? If not, I will send out the
new version tomorrow.
On Sun, Jun 08, 2014 at 09:10:15PM -0400, Chen, Gong wrote:
> BTW, any comments from other guys? Boris, Tony? If not, I will send out the
> new version tomorrow.
Looks ok at a first glance - I'll take a deeper look at your new version
with Steve's comments incorporated tomorrow - today's holiday here so no
work for me.
What dragged me to read emails I don't even remember. :-)
Thanks.