Re: [PATCH 2/3] cxl/region: Add sysfs attribute for locality attributes of CXL regions

[Brice Goglin <brice.goglin@gmail.com>]

Le 08/12/2023 à 00:31, Dave Jiang a écrit :

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index d879f5702cf2..72c47f624d63 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -645,6 +645,26 @@ static ssize_t size_show(struct device *dev, struct device_attribute *attr,
>   }
>   static DEVICE_ATTR_RW(size);
>   
> +#define ACCESS_ATTR(attrib)					\
> +static ssize_t attrib##_show(struct device *dev,		\
> +			   struct device_attribute *attr,	\
> +			   char *buf)				\
> +{								\
> +	struct cxl_region *cxlr = to_cxl_region(dev);		\
> +								\
> +	if (!cxlr->coord)					\
> +		return 0;					\
> +								\
> +	return sysfs_emit(buf, "%u\n",				\
> +			  cxlr->coord->attrib);			\
> +}								\

Hello

Latencies ares off by a factor of 1000 here (I see 586/686 for r/w attributes
for NUMA nodes in Qemu but 1000x higher for region attributes). For NUMA node attributes,
you're dividing latencies by a factor a 1000 in cxl_region_perf_attrs_callback():
  
         /* Adjust latencies from psec to nsec to be consistent with HMAT targets */
         coord = *cxlr->coord;
         coord.read_latency = DIV_ROUND_UP(coord.read_latency, 1000);
         coord.write_latency = DIV_ROUND_UP(coord.write_latency, 1000);

For region attributes, I think you're missing the same?

Brice