>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
>Sent: 03 November 2023 14:28
>To: Ira Weiny <ira.weiny@xxxxxxxxx>
>Cc: Dan Williams <dan.j.williams@xxxxxxxxx>; Smita Koralahalli
><Smita.KoralahalliChannabasappa@xxxxxxx>; Yazen Ghannam
><yazen.ghannam@xxxxxxx>; Davidlohr Bueso <dave@xxxxxxxxxxxx>; Dave
>Jiang <dave.jiang@xxxxxxxxx>; Alison Schofield <alison.schofield@xxxxxxxxx>;
>Vishal Verma <vishal.l.verma@xxxxxxxxx>; Ard Biesheuvel <ardb@xxxxxxxxxx>;
>linux-efi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
>cxl@xxxxxxxxxxxxxxx; Shiju Jose <shiju.jose@xxxxxxxxxx>
>Subject: Re: [PATCH RFC v3 1/6] cxl/trace: Remove uuid from event trace known
>events
>
>On Wed, 01 Nov 2023 14:11:18 -0700
>Ira Weiny <ira.weiny@xxxxxxxxx> wrote:
>
>> The uuid printed in the well known events is redundant. The uuid
>> defines what the event was.
>>
>> Remove the uuid from the known events and only report it in the
>> generic event as it remains informative there.
>>
>> Reviewed-by: Davidlohr Bueso <dave@xxxxxxxxxxxx>
>> Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>> Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>
>Removing the print is fine, but look like this also removes the actual trace point
>field. That's userspace ABI. Expanding it is fine, but taking fields away is more
>problematic.
>
>Are we sure we don't break anyone? Shiju, will rasdaemon be fine with this
>change?
The field hdr_uuid is removed from the common CXL_EVT_TP_entry shared by the
trace events cxl_generic_event, cxl_general_media, cxl_dram and cxl_memory_module .
rasdaemon will break because of this while processing these trace events
and also affects the corresponding error records in the SQLite data base.
Rasdaemon needs update to avoid this.
>
>Thanks,
>
>Jonathan
>
Thanks,
Shiju
>
>
>> ---
>> drivers/cxl/core/trace.h | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h index
>> a0b5819bc70b..79ed03637604 100644
>> --- a/drivers/cxl/core/trace.h
>> +++ b/drivers/cxl/core/trace.h
>> @@ -189,7 +189,6 @@ TRACE_EVENT(cxl_overflow,
>> __string(memdev, dev_name(&cxlmd->dev)) \
>> __string(host, dev_name(cxlmd->dev.parent)) \
>> __field(int, log) \
>> - __field_struct(uuid_t, hdr_uuid) \
>> __field(u64, serial) \
>> __field(u32, hdr_flags) \
>> __field(u16, hdr_handle) \
>> @@ -203,7 +202,6 @@ TRACE_EVENT(cxl_overflow,
>> __assign_str(host, dev_name((cxlmd)->dev.parent));
> \
>> __entry->log = (l);
> \
>> __entry->serial = (cxlmd)->cxlds->serial; \
>> - memcpy(&__entry->hdr_uuid, &(hdr).id, sizeof(uuid_t));
> \
>> __entry->hdr_length = (hdr).length;
> \
>> __entry->hdr_flags = get_unaligned_le24((hdr).flags);
> \
>> __entry->hdr_handle = le16_to_cpu((hdr).handle);
> \
>> @@ -212,12 +210,12 @@ TRACE_EVENT(cxl_overflow,
>> __entry->hdr_maint_op_class = (hdr).maint_op_class
>>
>> #define CXL_EVT_TP_printk(fmt, ...) \
>> - TP_printk("memdev=%s host=%s serial=%lld log=%s : time=%llu
>uuid=%pUb " \
>> + TP_printk("memdev=%s host=%s serial=%lld log=%s : time=%llu "
> \
>> "len=%d flags='%s' handle=%x related_handle=%x "
> \
>> "maint_op_class=%u : " fmt,
> \
>> __get_str(memdev), __get_str(host), __entry->serial,
> \
>> cxl_event_log_type_str(__entry->log),
> \
>> - __entry->hdr_timestamp, &__entry->hdr_uuid, __entry-
>>hdr_length,\
>> + __entry->hdr_timestamp, __entry->hdr_length,
> \
>> show_hdr_flags(__entry->hdr_flags), __entry->hdr_handle,
> \
>> __entry->hdr_related_handle, __entry->hdr_maint_op_class,
> \
>> ##__VA_ARGS__)
>> @@ -231,15 +229,17 @@ TRACE_EVENT(cxl_generic_event,
>>
>> TP_STRUCT__entry(
>> CXL_EVT_TP_entry
>> + __field_struct(uuid_t, hdr_uuid)
>> __array(u8, data, CXL_EVENT_RECORD_DATA_LENGTH)
>> ),
>>
>> TP_fast_assign(
>> CXL_EVT_TP_fast_assign(cxlmd, log, rec->hdr);
>> + memcpy(&__entry->hdr_uuid, &rec->hdr.id, sizeof(uuid_t));
>> memcpy(__entry->data, &rec->data,
>CXL_EVENT_RECORD_DATA_LENGTH);
>> ),
>>
>> - CXL_EVT_TP_printk("%s",
>> + CXL_EVT_TP_printk("uuid=%pUb %s", &__entry->hdr_uuid,
>> __print_hex(__entry->data,
>CXL_EVENT_RECORD_DATA_LENGTH)) );
>>
>>
