[ ... ]
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -298,30 +298,42 @@ TRACE_EVENT(non_standard_event,
  TRACE_EVENT(aer_event,
        TP_PROTO(const char *dev_name,
                 const u32 status,
-                const u8 severity),
+                const u8 severity,
+                const u32 tlp_header_valid,
+                struct aer_header_log_regs *tlp),
- TP_ARGS(dev_name, status, severity),
+       TP_ARGS(dev_name, status, severity, tlp_header_valid, tlp),
TP_STRUCT__entry(
                __string(       dev_name,       dev_name        )
                __field(        u32,            status          )
                __field(        u8,             severity        )
+               __field(        u32,            tlp_header_valid)

I'm guessing tlp_header_valid is just a boolean. It's after severity
which is just one byte long. Why not make this one byte as well,
otherwise you are wasting 3 bytes.

Thanks Steven. Yes boolean is fine.



+               __array(        u32,            buf, 4          )
        ),
TP_fast_assign(
                __assign_str(dev_name, dev_name);
                __entry->status              = status;
                __entry->severity    = severity;
+               __entry->tlp_header_valid = tlp_header_valid;

+               __entry->buf[0] = tlp->dw0;
+               __entry->buf[1] = tlp->dw1;
+               __entry->buf[2] = tlp->dw2;
+               __entry->buf[3] = tlp->dw3;

Should this assignment be dependent on whether or not tlp_header_valid
is true? Could tlp be pointing to some random memory if it isn't? What
about:

Good catch! will do so.


        if ((__entry->tlp_header_valid = tlp_header_valid)) {
                __entry->buf[0] = tlp->dw0;
                __entry->buf[1] = tlp->dw1;
                __entry->buf[2] = tlp->dw2;
                __entry->buf[3] = tlp->dw3;
        }


        ),
- TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
+       TP_printk("%s PCIe Bus Error: severity=%s, %s, TLP Header=%s\n",
                __get_str(dev_name),
                __entry->severity == AER_CORRECTABLE ? "Corrected" :
                        __entry->severity == AER_FATAL ?
                        "Fatal" : "Uncorrected, non-fatal",
                __entry->severity == AER_CORRECTABLE ?
                __print_flags(__entry->status, "|", aer_correctable_errors) :
-               __print_flags(__entry->status, "|", aer_uncorrectable_errors))
+               __print_flags(__entry->status, "|", aer_uncorrectable_errors),
+               __entry->tlp_header_valid ?
+                       __print_array(__entry->buf, 4, sizeof(unsigned int)) :

Note, "sizeof" will be shown in the format file that perf and trace-cmd
read to parse this event. They currently do not know how to parse that
(although I could add that in the future).

Since sizeof(unsigned int) is always 4 in Linux, just use 4.

Yes, I will make the aboves changes and send out V2 patch. Thank you for your time and comments.

Thomas


                        __print_array(__entry->buf, 4, 4)

-- Steve


+                       "Not available")
  );
/*
--
2.14.1

Reply via email to