Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

From: Alison Schofield <alison.schofield@intel.com>
To: Ira Weiny <ira.weiny@intel.com>, Steven Rostedt <rostedt@goodmis.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	linux-cxl@vger.kernel.org
Subject: Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events
Date: Tue, 16 Apr 2024 17:39:40 -0700	[thread overview]
Message-ID: <Zh8aTGIBOYW5Hl/n@aschofie-mobl2> (raw)
In-Reply-To: <661eaee1cadc2_3f1829294fe@iweiny-mobl.notmuch>

+ Steve 

On Tue, Apr 16, 2024 at 10:01:21AM -0700, Ira Weiny wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> 
> [snip]
> 
> > diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> > index 161bdb5734b0..6ad4998aeb9a 100644
> > --- a/drivers/cxl/core/trace.h
> > +++ b/drivers/cxl/core/trace.h
> > @@ -14,6 +14,22 @@
> >  #include <cxlmem.h>
> >  #include "core.h"
> > 

snip

> > +#define store_region_info(cxlmd, dpa, entry_uuid, entry_hpa)		\
> > +	do {								\
> > +		struct cxl_region *cxlr;				\
> > +									\
> > +		cxlr = cxl_dpa_to_region(cxlmd, dpa);			\
> > +		if (cxlr) {						\
> > +			uuid_copy(&(entry_uuid), &cxlr->params.uuid);	\
> > +			entry_hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);	\
> > +		} else {						\
> 
> Does the record get zeroed such that uuid is 0 here?

It appears in my usage, but I don't know for sure. 
CC'ing Steve to tell me if the TP_PROTO macro initializes the
__field_struct to zero.

Left in the snippets below that shows its definition...

> 
> > +			entry_hpa = ULLONG_MAX;				\
> > +		}							\
> > +	} while (0)

snip

> >  TRACE_EVENT(cxl_general_media,
> >  
> >  	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
> > @@ -330,10 +349,13 @@ TRACE_EVENT(cxl_general_media,
> >  		__field(u8, channel)
> >  		__field(u32, device)
> >  		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
> > -		__field(u16, validity_flags)
> >  		/* Following are out of order to pack trace record */
> > +		__field(u64, hpa)
> > +		__field_struct(uuid_t, region_uuid)
> > +		__field(u16, validity_flags)
> >  		__field(u8, rank)
> >  		__field(u8, dpa_flags)
> > +		__string(region_name, to_region_name(cxlmd, to_gm_dpa(record)))
> >  	),
> >  
> >  	TP_fast_assign(
> > @@ -354,18 +376,23 @@ TRACE_EVENT(cxl_general_media,
> >  		memcpy(__entry->comp_id, &rec->component_id,
> >  			CXL_EVENT_GEN_MED_COMP_ID_SIZE);
> >  		__entry->validity_flags = get_unaligned_le16(&rec->validity_flags);
> > +		__assign_str(region_name, to_region_name(cxlmd, to_gm_dpa(record)));
> > +		store_region_info(cxlmd, to_gm_dpa(record),
> > +				  __entry->region_uuid, __entry->hpa);
> >  	),
> >  
> >  	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \
> >  		"descriptor='%s' type='%s' transaction_type='%s' channel=%u rank=%u " \
> > -		"device=%x comp_id=%s validity_flags='%s'",
> > +		"device=%x comp_id=%s validity_flags='%s' " \
> > +		"hpa=%llx region=%s region_uuid=%pUb",
> >  		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
> >  		show_event_desc_flags(__entry->descriptor),
> >  		show_mem_event_type(__entry->type),
> >  		show_trans_type(__entry->transaction_type),
> >  		__entry->channel, __entry->rank, __entry->device,
> >  		__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
> > -		show_valid_flags(__entry->validity_flags)
> > +		show_valid_flags(__entry->validity_flags),
> > +		__entry->hpa, __get_str(region_name), &__entry->region_uuid
> >  	)
> >  );
> >  