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

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

On Mon, Apr 22, 2024 at 09:23:32PM -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..2e24364b2b8d 100644
> > --- a/drivers/cxl/core/trace.h
> > +++ b/drivers/cxl/core/trace.h
> > @@ -14,6 +14,28 @@
> >  #include <cxlmem.h>
> >  #include "core.h"
> >  
> > +#ifndef __CXL_EVENTS_DECLARE_ONCE_ONLY
> > +#define __CXL_EVENTS_DECLARE_ONCE_ONLY
> > +static inline
> > +void store_region_info(const struct cxl_memdev *cxlmd, u64 dpa, uuid_t *uuid,
> > +		       u64 *hpa)
> > +{
> > +	struct cxl_region *cxlr;
> > +
> > +	cxlr = cxl_dpa_to_region(cxlmd, dpa);
> 
> This would normally be:  (somewhat nitty though...)
> 
> 	if (!cxlr) {
> 		uuid_copy(uuid, &uuid_null);
> 		*hpa = ULLONG_MAX;
> 		return;
> 	}
> 
> 	uuid_copy(uuid, &cxlr->params.uuid);
> 	*hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);
> }
> 
> But in this context I don't think it is critical.

Thanks for pointing out. When I switch to inline I didn't
'see' that. I'd like to fix it up. Let me see what else 
comes in.

--Alison

> 
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> 
> 
> [snip]