2024-05-31 02:49:27

by Huang, Ying

[permalink] [raw]
Subject: [PATCH] cxl/region: Support to calculate memory tier abstract distance

To place memory nodes backed by CXL regions in the appropriate memory
tiers. So that, pages can be promoted/demoted with the existing
memory tiering mechanism.

The abstract distance is calculated based on the memory access latency
and bandwidth of CXL regions. Which in turn comes from the HMAT
and CDAT, etc.

Signed-off-by: "Huang, Ying" <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Bharata B Rao <[email protected]>
Cc: Alistair Popple <[email protected]>
Cc: Aneesh Kumar K.V <[email protected]>
---
drivers/cxl/core/region.c | 40 +++++++++++++++++++++++++++++++++++----
drivers/cxl/cxl.h | 1 +
2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 00a9f0eef8dd..1f8f71a034ae 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);
+}
+
static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
unsigned long action, void *arg)
{
struct cxl_region *cxlr = container_of(nb, struct cxl_region,
memory_notifier);
- struct cxl_region_params *p = &cxlr->params;
- struct cxl_endpoint_decoder *cxled = p->targets[0];
- struct cxl_decoder *cxld = &cxled->cxld;
struct memory_notify *mnb = arg;
int nid = mnb->status_change_nid;
int region_nid;
@@ -2319,7 +2326,7 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
if (nid == NUMA_NO_NODE || action != MEM_ONLINE)
return NOTIFY_DONE;

- region_nid = phys_to_target_node(cxld->hpa_range.start);
+ region_nid = cxl_region_nid(cxlr);
if (nid != region_nid)
return NOTIFY_DONE;

@@ -2329,6 +2336,27 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
return NOTIFY_OK;
}

+static int cxl_region_calculate_adistance(struct notifier_block *nb,
+ unsigned long nid, void *data)
+{
+ struct cxl_region *cxlr = container_of(nb, struct cxl_region,
+ adist_notifier);
+ int region_nid;
+ struct access_coordinate *perf;
+ int *adist = data;
+
+ region_nid = cxl_region_nid(cxlr);
+ if (nid != region_nid)
+ return NOTIFY_OK;
+
+ perf = &cxlr->coord[ACCESS_COORDINATE_CPU];
+
+ if (mt_perf_to_adistance(perf, adist))
+ return NOTIFY_OK;
+
+ return NOTIFY_STOP;
+}
+
/**
* devm_cxl_add_region - Adds a region to a decoder
* @cxlrd: root decoder
@@ -2380,6 +2408,10 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
cxlr->memory_notifier.priority = CXL_CALLBACK_PRI;
register_memory_notifier(&cxlr->memory_notifier);

+ cxlr->adist_notifier.notifier_call = cxl_region_calculate_adistance;
+ cxlr->adist_notifier.priority = 100;
+ register_mt_adistance_algorithm(&cxlr->adist_notifier);
+
rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr);
if (rc)
return ERR_PTR(rc);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 603c0120cff8..6891f87f8ef7 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -534,6 +534,7 @@ struct cxl_region {
struct cxl_region_params params;
struct access_coordinate coord[ACCESS_COORDINATE_MAX];
struct notifier_block memory_notifier;
+ struct notifier_block adist_notifier;
};

struct cxl_nvdimm_bridge {
--
2.39.2



2024-06-05 12:56:59

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] cxl/region: Support to calculate memory tier abstract distance

On Fri, 31 May 2024 10:48:52 +0800
Huang Ying <[email protected]> wrote:

> To place memory nodes backed by CXL regions in the appropriate memory
> tiers. So that, pages can be promoted/demoted with the existing
> memory tiering mechanism.
>
> The abstract distance is calculated based on the memory access latency
> and bandwidth of CXL regions. Which in turn comes from the HMAT
> and CDAT, etc.
>
> Signed-off-by: "Huang, Ying" <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Jonathan Cameron <[email protected]>
> Cc: Dave Jiang <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Bharata B Rao <[email protected]>
> Cc: Alistair Popple <[email protected]>
> Cc: Aneesh Kumar K.V <[email protected]>

Minor comments inline. Otherwise, looks good to me.

Jonathan


> ---
> drivers/cxl/core/region.c | 40 +++++++++++++++++++++++++++++++++++----
> drivers/cxl/cxl.h | 1 +
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 00a9f0eef8dd..1f8f71a034ae 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);
> +}
> +
> static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
> unsigned long action, void *arg)
> {
> struct cxl_region *cxlr = container_of(nb, struct cxl_region,
> memory_notifier);
> - struct cxl_region_params *p = &cxlr->params;
> - struct cxl_endpoint_decoder *cxled = p->targets[0];
> - struct cxl_decoder *cxld = &cxled->cxld;
> struct memory_notify *mnb = arg;
> int nid = mnb->status_change_nid;
> int region_nid;
> @@ -2319,7 +2326,7 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
> if (nid == NUMA_NO_NODE || action != MEM_ONLINE)
> return NOTIFY_DONE;
>
> - region_nid = phys_to_target_node(cxld->hpa_range.start);
> + region_nid = cxl_region_nid(cxlr);
> if (nid != region_nid)
> return NOTIFY_DONE;
>
> @@ -2329,6 +2336,27 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
> return NOTIFY_OK;
> }
>
> +static int cxl_region_calculate_adistance(struct notifier_block *nb,
> + unsigned long nid, void *data)
> +{
> + struct cxl_region *cxlr = container_of(nb, struct cxl_region,
> + adist_notifier);
> + int region_nid;
> + struct access_coordinate *perf;
> + int *adist = data;
Local style looks to be reverse xmas tree subject to any dependencies etc.

> +
> + region_nid = cxl_region_nid(cxlr);
> + if (nid != region_nid)
> + return NOTIFY_OK;
> +
> + perf = &cxlr->coord[ACCESS_COORDINATE_CPU];
> +
> + if (mt_perf_to_adistance(perf, adist))
> + return NOTIFY_OK;
> +
> + return NOTIFY_STOP;
> +}
> +
> /**
> * devm_cxl_add_region - Adds a region to a decoder
> * @cxlrd: root decoder
> @@ -2380,6 +2408,10 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
> cxlr->memory_notifier.priority = CXL_CALLBACK_PRI;
> register_memory_notifier(&cxlr->memory_notifier);
>
> + cxlr->adist_notifier.notifier_call = cxl_region_calculate_adistance;
> + cxlr->adist_notifier.priority = 100;
> + register_mt_adistance_algorithm(&cxlr->adist_notifier);
> +
> rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr);
> if (rc)
> return ERR_PTR(rc);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 603c0120cff8..6891f87f8ef7 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -534,6 +534,7 @@ struct cxl_region {
This has docs that need an updte.
> struct cxl_region_params params;
> struct access_coordinate coord[ACCESS_COORDINATE_MAX];
> struct notifier_block memory_notifier;
> + struct notifier_block adist_notifier;
> };
>
> struct cxl_nvdimm_bridge {


2024-06-06 01:17:19

by Huang, Ying

[permalink] [raw]
Subject: Re: [PATCH] cxl/region: Support to calculate memory tier abstract distance

Hi, Jonathan,

Thank you very much for review!

Jonathan Cameron <[email protected]> writes:

> On Fri, 31 May 2024 10:48:52 +0800
> Huang Ying <[email protected]> wrote:
>
>> To place memory nodes backed by CXL regions in the appropriate memory
>> tiers. So that, pages can be promoted/demoted with the existing
>> memory tiering mechanism.
>>
>> The abstract distance is calculated based on the memory access latency
>> and bandwidth of CXL regions. Which in turn comes from the HMAT
>> and CDAT, etc.
>>
>> Signed-off-by: "Huang, Ying" <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Jonathan Cameron <[email protected]>
>> Cc: Dave Jiang <[email protected]>
>> Cc: Dan Williams <[email protected]>
>> Cc: Bharata B Rao <[email protected]>
>> Cc: Alistair Popple <[email protected]>
>> Cc: Aneesh Kumar K.V <[email protected]>
>
> Minor comments inline. Otherwise, looks good to me.
>
> Jonathan
>
>
>> ---
>> drivers/cxl/core/region.c | 40 +++++++++++++++++++++++++++++++++++----
>> drivers/cxl/cxl.h | 1 +
>> 2 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 00a9f0eef8dd..1f8f71a034ae 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);
>> +}
>> +
>> static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
>> unsigned long action, void *arg)
>> {
>> struct cxl_region *cxlr = container_of(nb, struct cxl_region,
>> memory_notifier);
>> - struct cxl_region_params *p = &cxlr->params;
>> - struct cxl_endpoint_decoder *cxled = p->targets[0];
>> - struct cxl_decoder *cxld = &cxled->cxld;
>> struct memory_notify *mnb = arg;
>> int nid = mnb->status_change_nid;
>> int region_nid;
>> @@ -2319,7 +2326,7 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
>> if (nid == NUMA_NO_NODE || action != MEM_ONLINE)
>> return NOTIFY_DONE;
>>
>> - region_nid = phys_to_target_node(cxld->hpa_range.start);
>> + region_nid = cxl_region_nid(cxlr);
>> if (nid != region_nid)
>> return NOTIFY_DONE;
>>
>> @@ -2329,6 +2336,27 @@ static int cxl_region_perf_attrs_callback(struct notifier_block *nb,
>> return NOTIFY_OK;
>> }
>>
>> +static int cxl_region_calculate_adistance(struct notifier_block *nb,
>> + unsigned long nid, void *data)
>> +{
>> + struct cxl_region *cxlr = container_of(nb, struct cxl_region,
>> + adist_notifier);
>> + int region_nid;
>> + struct access_coordinate *perf;
>> + int *adist = data;
> Local style looks to be reverse xmas tree subject to any dependencies etc.

Sure. Will change this to follow reverse xmas tree.

>> +
>> + region_nid = cxl_region_nid(cxlr);
>> + if (nid != region_nid)
>> + return NOTIFY_OK;
>> +
>> + perf = &cxlr->coord[ACCESS_COORDINATE_CPU];
>> +
>> + if (mt_perf_to_adistance(perf, adist))
>> + return NOTIFY_OK;
>> +
>> + return NOTIFY_STOP;
>> +}
>> +
>> /**
>> * devm_cxl_add_region - Adds a region to a decoder
>> * @cxlrd: root decoder
>> @@ -2380,6 +2408,10 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>> cxlr->memory_notifier.priority = CXL_CALLBACK_PRI;
>> register_memory_notifier(&cxlr->memory_notifier);
>>
>> + cxlr->adist_notifier.notifier_call = cxl_region_calculate_adistance;
>> + cxlr->adist_notifier.priority = 100;
>> + register_mt_adistance_algorithm(&cxlr->adist_notifier);
>> +
>> rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr);
>> if (rc)
>> return ERR_PTR(rc);
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index 603c0120cff8..6891f87f8ef7 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -534,6 +534,7 @@ struct cxl_region {
> This has docs that need an updte.

Yes. Thanks for reminding!

>> struct cxl_region_params params;
>> struct access_coordinate coord[ACCESS_COORDINATE_MAX];
>> struct notifier_block memory_notifier;
>> + struct notifier_block adist_notifier;
>> };
>>
>> struct cxl_nvdimm_bridge {

--
Best Regards,
Huang, Ying