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

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Apr 23, 2024 at 10:33:05PM -0700, Dan Williams wrote:
> alison.schofield@ 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>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  drivers/cxl/core/mbox.c   |  5 ++---
> >  drivers/cxl/core/memdev.c |  8 ++++----
> >  drivers/cxl/core/region.c |  8 ++++----
> >  drivers/cxl/core/trace.h  | 27 +++++++++++----------------
> >  drivers/cxl/cxlmem.h      |  3 +--
> >  5 files changed, 22 insertions(+), 29 deletions(-)
> 
> A lot of the thrash here is about removing cxlr as an argument, and per
> the comment on patch3 I think it is cleaner to go the other way and
> always pass it in.

I still think we can mimic what is done for general_media and dram
trace events. ie. lookup the region only if trace_cxl_poison_enabled().
as you suggested in previous patch.

I can see why you call it thrash. I'll counter that with the fact that
it is a fairly easy to read diff, and the fact that removing the passed
along cxlr param, is pretty much 'tested' when it compiles.

-- Alison