Re: [PATCH -V2] cxl/region: Support to calculate memory tier abstract distance

[Alison Schofield]

On Wed, Jun 12, 2024 at 10:09:14AM +0800, Ying Huang wrote:

snip

> >> ---
> >>  drivers/cxl/core/region.c | 40 +++++++++++++++++++++++++++++++++++----
> >>  drivers/cxl/cxl.h         |  2 ++
> >>  2 files changed, 38 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> >> index 3c2b6144be23..81d0910c0a02 100644
> >> --- a/drivers/cxl/core/region.c
> >> +++ b/drivers/cxl/core/region.c
> >> @@ -9,6 +9,7 @@
> >>  #include <linux/uuid.h>
> >>  #include <linux/sort.h>
> >>  #include <linux/idr.h>
> >> +#include <linux/memory-tiers.h>
> >>  #include <cxlmem.h>
> >>  #include <cxl.h>
> >>  #include "core.h"
> >> @@ -2304,14 +2305,20 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
> >>  	return true;
> >>  }
> >>  
> >> +static int cxl_region_nid(struct cxl_region *cxlr)
> >> +{
> >> +	struct cxl_region_params *p = &cxlr->params;
> >> +	struct cxl_endpoint_decoder *cxled = p->targets[0];
> >> +	struct cxl_decoder *cxld = &cxled->cxld;
> >> +
> >> +	return phys_to_target_node(cxld->hpa_range.start);
> >> +}
> >> +
> >
> > I believe it's OK to send a resource_size_t to phys_to_target_node()
> > like this:
> >
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -2308,10 +2308,8 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
> >  static int cxl_region_nid(struct cxl_region *cxlr)
> >  {
> >         struct cxl_region_params *p = &cxlr->params;
> > -       struct cxl_endpoint_decoder *cxled = p->targets[0];
> > -       struct cxl_decoder *cxld = &cxled->cxld;
> >
> > -       return phys_to_target_node(cxld->hpa_range.start);
> > +       return phys_to_target_node(p->res->start);
> >  }
> >
> 
> I believe this works.  But the original implementation is just a
> mechanical code movement from cxl_region_perf_attrs_callback().  So, I
> prefer to keep it stupid. Then, further optimization can be done on top
> of it.  Is it good for you?

I prefer to do it now while we are thinking about it.

How about a precursor patch:
Patch 1/2: cxl/region: Add a region to node id helper

--and then in that commit log you can say it's a simplified lookup 
and is being done in preparation for adding another caller.

-- Alison

> 
snip
> >