Re: [PATCH 4/4] cxl/core: Remove cxlr dependency from cxl_poison trace events

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 27 Mar 2024 21:36:33 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> cxl_poison trace events pass a pointer to a struct cxl_region
> when poison is discovered in a mapped endpoint. This made for
> easy look up of region name, uuid, and was useful in starting
> the dpa->hpa translation.
> 
> Another set of trace helpers is now available that eliminates
> the need to pass on that cxlr. (It was passed along a lot!)
> 
> In addition to tidying up the cxl_poison calling path, shifting
> to the new helpers also means all CXL trace events will share
> the same code in that regard.
> 
> Switch from a uuid array to the field_struct type uuid_t to
> align on one uuid format in all CXL trace events. That is useful
> when sharing region uuid lookup helpers.
> 
> No externally visible naming changes are made to cxl_poison trace
> events.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Trivial formatting comment inline.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 6ad4998aeb9a..2cd66c04602a 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -681,23 +681,23 @@ TRACE_EVENT(cxl_memory_module,
>  

> @@ -712,27 +712,20 @@ TRACE_EVENT(cxl_poison,
>  		__entry->source = cxl_poison_record_source(record);
>  		__entry->trace_type = trace_type;
>  		__entry->flags = flags;
> -		if (cxlr) {
> -			__assign_str(region, dev_name(&cxlr->dev));
> -			memcpy(__entry->uuid, &cxlr->params.uuid, 16);
> -			__entry->hpa = cxl_trace_hpa(cxlr, cxlmd,
> -						     __entry->dpa);
> -		} else {
> -			__assign_str(region, "");
> -			memset(__entry->uuid, 0, 16);
> -			__entry->hpa = ULLONG_MAX;
> -		}
> +		__assign_str(region, to_region_name(cxlmd, cxl_poison_record_dpa(record)));
> +		store_region_info(cxlmd, __entry->dpa, __entry->uuid,
> +				  __entry->hpa);
>  	    ),
>  
> -	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s "  \
> -		"region_uuid=%pU hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \
> +	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s"  \
> +		"region_uuid=%pUb hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \

the spaces before the \ could do with tidying up.  

>  		"source=%s flags=%s overflow_time=%llu",
>  		__get_str(memdev),
>  		__get_str(host),
>  		__entry->serial,
>  		show_poison_trace_type(__entry->trace_type),
>  		__get_str(region),
> -		__entry->uuid,
> +		&__entry->uuid,
>  		__entry->hpa,
>  		__entry->dpa,
>  		__entry->dpa_length,