Messages in this thread | | | Date | Sun, 16 Oct 2022 14:43:33 -0700 | From | Ira Weiny <> | Subject | Re: [RFC V2 PATCH 02/11] cxl/mem: Implement Get Event Records command |
| |
On Sat, Oct 15, 2022 at 07:28:40AM -0400, Steven Rostedt wrote: > On Mon, 10 Oct 2022 15:41:22 -0700 > ira.weiny@intel.com wrote: > > > new file mode 100644 > > index 000000000000..318ba5fe046e > > --- /dev/null > > +++ b/include/trace/events/cxl.h > > @@ -0,0 +1,130 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#undef TRACE_SYSTEM > > +#define TRACE_SYSTEM cxl > > + > > +#if !defined(_CXL_TRACE_EVENTS_H) || defined(TRACE_HEADER_MULTI_READ) > > +#define _CXL_TRACE_EVENTS_H > > + > > +#include <asm-generic/unaligned.h> > > +#include <linux/tracepoint.h> > > + > > +TRACE_EVENT(overflow, > > Please do not use generic names for grouped events. As most tooling can > use the name and not associate it with the group, it makes it ambiguous > for what event it wants to enable. > > That is, call it "cxl_overflow" and not "overflow".
I did not realize that.[1]
[1] https://lore.kernel.org/linux-cxl/67c4e0da-87e3-44a1-1902-220dbc312700@intel.com/
> > > + > > + TP_PROTO(const char *dev_name, enum cxl_event_log_type log, > > + struct cxl_get_event_payload *payload), > > + > > + TP_ARGS(dev_name, log, payload), > > + > > + TP_STRUCT__entry( > > + __string(dev_name, dev_name) > > + __field(int, log) > > + __field(u64, first) > > + __field(u64, last) > > + __field(u16, count) > > + ), > > + > > + TP_fast_assign( > > + __assign_str(dev_name, dev_name); > > + __entry->log = log; > > + __entry->count = le16_to_cpu(payload->overflow_err_count); > > + __entry->first = le64_to_cpu(payload->first_overflow_timestamp); > > + __entry->last = le64_to_cpu(payload->last_overflow_timestamp); > > + ), > > + > > + TP_printk("%s: EVENT LOG OVERFLOW log=%s : %u records from %llu to %llu", > > + __get_str(dev_name), cxl_event_log_type_str(__entry->log), > > + __entry->count, __entry->first, __entry->last) > > + > > +); > > + > > > +TRACE_EVENT(generic_event, > > Same here. It's a "cxl_generic_event" not a "generic_event" that will > clash with any other subsystem that would (but shouldn't) use the same > name.
Sure I'll change it back.
Thanks for setting us straight, Ira
> > -- Steve > > > + > > + TP_PROTO(const char *dev_name, enum cxl_event_log_type log, > > + struct cxl_event_record_raw *rec), > > + > > + TP_ARGS(dev_name, log, rec), > > + > > + TP_STRUCT__entry( > > + CXL_EVT_TP_entry > > + __array(u8, data, CXL_EVENT_RECORD_DATA_LENGTH) > > + ), > > + > > + TP_fast_assign( > > + CXL_EVT_TP_fast_assign(dev_name, log, rec->hdr); > > + memcpy(__entry->data, &rec->data, CXL_EVENT_RECORD_DATA_LENGTH); > > + ), > > + > > + CXL_EVT_TP_printk("%s", > > + __print_hex(__entry->data, CXL_EVENT_RECORD_DATA_LENGTH)) > > +); > > + > > +#endif /* _CXL_TRACE_EVENTS_H */ > >
| |