Re: [NDCTL PATCH v3 2/3] ndctl: cxl: Add QoS class support for the memory device

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

On Wed, Jan 24, 2024 at 01:54:47PM -0700, Dave Jiang wrote:
> Add libcxl API to retrieve the QoS class tokens for the memory
> devices. Two API calls are added. One for 'ram' or 'volatile'
> mode and another for 'pmem' or 'persistent' mode. Support also added
> for displaying the QoS class tokens through the 'cxl list' command.
> There can be 1 or more QoS class tokens for the memory device if
> they are valid. The qos_class tokens are displayed behind -vvv
> verbose level.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

One tidbit below. 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>


> ---
> v3:
> - Rebase to pending branch
> - Skip from failing if no qos_class sysfs attrib found
> ---
>  cxl/json.c         |   36 +++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.c   |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |    2 ++
>  cxl/lib/private.h  |    2 ++
>  cxl/libcxl.h       |    7 +++++++
>  5 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 48a43ddf14b0..dcbac8c14f03 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -770,12 +770,32 @@ err_free:
>  	return jpoison;
>  }
>  
> +static struct json_object *get_qos_json_object(struct json_object *jdev,
> +					       struct qos_class *qos_class)
> +{
> +	struct json_object *jqos_array = json_object_new_array();
> +	struct json_object *jobj;
> +	int i;
> +
> +	if (!jqos_array)
> +		return NULL;
> +
> +	for (i = 0; i < qos_class->nr; i++) {
> +		jobj = json_object_new_int(qos_class->qos[i]);
> +		if (jobj)
> +			json_object_array_add(jqos_array, jobj);
> +	}
> +
> +	return jqos_array;
> +}
> +
>  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		unsigned long flags)
>  {
>  	const char *devname = cxl_memdev_get_devname(memdev);
> -	struct json_object *jdev, *jobj;
> +	struct json_object *jdev, *jobj, *jqos;

Can the generic *jobj be used below rather than adding the new *jqos?


>  	unsigned long long serial, size;
> +	struct qos_class *qos_class;
>  	int numa_node;
>  
>  	jdev = json_object_new_object();
> @@ -791,6 +811,13 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		jobj = util_json_object_size(size, flags);
>  		if (jobj)
>  			json_object_object_add(jdev, "pmem_size", jobj);
> +
> +		if (flags & UTIL_JSON_QOS_CLASS) {
> +			qos_class = cxl_memdev_get_pmem_qos_class(memdev);
> +			jqos = get_qos_json_object(jdev, qos_class);
> +			if (jqos)
> +				json_object_object_add(jdev, "pmem_qos_class", jqos);
> +		}
>  	}
>  
>  	size = cxl_memdev_get_ram_size(memdev);
> @@ -798,6 +825,13 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		jobj = util_json_object_size(size, flags);
>  		if (jobj)
>  			json_object_object_add(jdev, "ram_size", jobj);
> +
> +		if (flags & UTIL_JSON_QOS_CLASS) {
> +			qos_class = cxl_memdev_get_ram_qos_class(memdev);
> +			jqos = get_qos_json_object(jdev, qos_class);
> +			if (jqos)
> +				json_object_object_add(jdev, "ram_qos_class", jqos);
> +		}
>  	}
>  
>  	if (flags & UTIL_JSON_HEALTH) {

snip

> 
>