Received-SPF: pass (google.com: domain of linux-kernel+bounces-212411-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Huang, Ying" <ying.huang@intel.com>
To: Alison Schofield <alison.schofield@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>,  <linux-cxl@vger.kernel.org>,
  <linux-kernel@vger.kernel.org>,  Andrew Morton
 <akpm@linux-foundation.org>,  Jonathan Cameron
 <Jonathan.Cameron@huawei.com>,  Dave Jiang <dave.jiang@intel.com>,
  Bharata B Rao <bharata@amd.com>,  Alistair Popple <apopple@nvidia.com>,
  "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
Subject: Re: [PATCH -V2] cxl/region: Support to calculate memory tier
 abstract distance
In-Reply-To: <Zmnc/bIkAQp6dpxQ@aschofie-mobl2> (Alison Schofield's message of
	"Wed, 12 Jun 2024 10:38:05 -0700")
References: <20240611055423.470574-1-ying.huang@intel.com>
	<ZmjBfcaosIlOODFR@aschofie-mobl2>
	<87sexi7vzp.fsf@yhuang6-desk2.ccr.corp.intel.com>
	<Zmnc/bIkAQp6dpxQ@aschofie-mobl2>
Date: Thu, 13 Jun 2024 08:40:04 +0800
Message-ID: <87ikyd7k0r.fsf@yhuang6-desk2.ccr.corp.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii

Alison Schofield <alison.schofield@intel.com> writes:

> 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.

This works.  Will do it.

--
Best Regards,
Huang, Ying