diff mbox series

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

Message ID 170441210897.3574076.3084661576808646327.stgit@djiang5-mobl3
State Superseded
Headers show
Series cxl: Add support to report region access coordinates to numa nodes | expand

Commit Message

Dave Jiang Jan. 4, 2024, 11:48 p.m. UTC
Add read/write latencies and bandwidth sysfs attributes for the enabled CXL
region. The bandwidth is the aggregated bandwidth of all devices that
contribute to the CXL region. The latency is the worst latency of the
device amongst all the devices that contribute to the CXL region.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- Make attribs not visible if no data (Jonathan)
- Check against coord.attrib (Jonathan)
- Fix documentation verbiage (Jonathan)
---
 Documentation/ABI/testing/sysfs-bus-cxl |   60 +++++++++++++++++++++++++++++++
 drivers/cxl/core/region.c               |   40 +++++++++++++++++++++
 2 files changed, 100 insertions(+)

Comments

Dan Williams Jan. 5, 2024, 12:19 a.m. UTC | #1
Dave Jiang wrote:
> Add read/write latencies and bandwidth sysfs attributes for the enabled CXL
> region. The bandwidth is the aggregated bandwidth of all devices that
> contribute to the CXL region. The latency is the worst latency of the
> device amongst all the devices that contribute to the CXL region.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v3:
> - Make attribs not visible if no data (Jonathan)
> - Check against coord.attrib (Jonathan)
> - Fix documentation verbiage (Jonathan)
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |   60 +++++++++++++++++++++++++++++++
>  drivers/cxl/core/region.c               |   40 +++++++++++++++++++++
>  2 files changed, 100 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index fff2581b8033..86d3dbe12129 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -552,3 +552,63 @@ Description:
>  		attribute is only visible for devices supporting the
>  		capability. The retrieved errors are logged as kernel
>  		events when cxl_poison event tracing is enabled.
> +
> +
> +What:		/sys/bus/cxl/devices/regionZ/read_bandwidth
> +Date:		Jan, 2024
> +KernelVersion:	v6.9
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) The aggregated read bandwidth of the region. The number is
> +		the accumulated read bandwidth of all CXL memory devices that
> +		contributes to the region in MB/s. Should be equivalent to

Perhaps:

s|Should be equivalent|It the identical data that would appear in
/sys/devices/system/node/nodeX/access0/initiators/read_bandwidth if /
when this region is onlined as memory nodeX|.

Note that I did a s/accessY/access0/ since this data is relative to a
single Generic Port. The performance from other intiators that go
through an alternate port is not represented, right?

> +		attributes in
> +		/sys/devices/system/node/nodeX/accessY/initiators/read_bandwidth.
> +		See Documentation/ABI/stable/sysfs-devices-node. The generic
> +		target bandwidth that is part of the whole path calculation is
> +		the best performance latency provided by the HMAT SSLBIS table.

Shouldn't this be s/HMAT SSBLIS/CDAT DSBLIS/ throughout?

...but maybe just delete it because this number is pretty far removed
from the CDAT data. The data has been munged by considering the
intervening topology, and is aggregated across all the endpoints.
Jonathan Cameron Jan. 8, 2024, 12:07 p.m. UTC | #2
On Thu, 4 Jan 2024 16:19:45 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Dave Jiang wrote:
> > Add read/write latencies and bandwidth sysfs attributes for the enabled CXL
> > region. The bandwidth is the aggregated bandwidth of all devices that
> > contribute to the CXL region. The latency is the worst latency of the
> > device amongst all the devices that contribute to the CXL region.
> > 
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> > v3:
> > - Make attribs not visible if no data (Jonathan)
> > - Check against coord.attrib (Jonathan)
> > - Fix documentation verbiage (Jonathan)
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cxl |   60 +++++++++++++++++++++++++++++++
> >  drivers/cxl/core/region.c               |   40 +++++++++++++++++++++
> >  2 files changed, 100 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> > index fff2581b8033..86d3dbe12129 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -552,3 +552,63 @@ Description:
> >  		attribute is only visible for devices supporting the
> >  		capability. The retrieved errors are logged as kernel
> >  		events when cxl_poison event tracing is enabled.
> > +
> > +
> > +What:		/sys/bus/cxl/devices/regionZ/read_bandwidth
> > +Date:		Jan, 2024
> > +KernelVersion:	v6.9
> > +Contact:	linux-cxl@vger.kernel.org
> > +Description:
> > +		(RO) The aggregated read bandwidth of the region. The number is
> > +		the accumulated read bandwidth of all CXL memory devices that
> > +		contributes to the region in MB/s. Should be equivalent to  
> 
> Perhaps:
> 
> s|Should be equivalent|It the identical data that would appear in
> /sys/devices/system/node/nodeX/access0/initiators/read_bandwidth if /
> when this region is onlined as memory nodeX|.
> 
> Note that I did a s/accessY/access0/ since this data is relative to a
> single Generic Port. The performance from other intiators that go
> through an alternate port is not represented, right?
Hi Dan,

You've lost me on this comment.  access0 is the access class, not enumerating a
particular port etc. This interface always gives me a headache (and is
at least partly my fault :(! but I think this is specifying the access
characteristics from the nearest initiator (CPUs and others) to the port.
access1 would be from a CPU to the port.

The actual initiators are nodeY in
/sys/devices/system/node/nodeX/access0/initiators/nodeY etc

Jonathan
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index fff2581b8033..86d3dbe12129 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -552,3 +552,63 @@  Description:
 		attribute is only visible for devices supporting the
 		capability. The retrieved errors are logged as kernel
 		events when cxl_poison event tracing is enabled.
+
+
+What:		/sys/bus/cxl/devices/regionZ/read_bandwidth
+Date:		Jan, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The aggregated read bandwidth of the region. The number is
+		the accumulated read bandwidth of all CXL memory devices that
+		contributes to the region in MB/s. Should be equivalent to
+		attributes in
+		/sys/devices/system/node/nodeX/accessY/initiators/read_bandwidth.
+		See Documentation/ABI/stable/sysfs-devices-node. The generic
+		target bandwidth that is part of the whole path calculation is
+		the best performance latency provided by the HMAT SSLBIS table.
+
+
+What:		/sys/bus/cxl/devices/regionZ/write_bandwidth
+Date:		Jan, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The aggregated write bandwidth of the region. The number is
+		the accumulated write bandwidth of all CXL memory devices that
+		contributes to the region in MB/s. Should be equivalent to
+		attributes in
+		/sys/devices/system/node/nodeX/accessY/initiators/write_bandwidth.
+		See Documentation/ABI/stable/sysfs-devices-node. The generic
+		target bandwidth that is part of the whole path calculation is
+		the best performance latency provided by the HMAT SSLBIS table.
+
+
+What:		/sys/bus/cxl/devices/regionZ/read_latency
+Date:		Jan, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The read latency of the region. The number is
+		the worst read latency of all CXL memory devices that
+		contributes to the region in nanoseconds. Should be
+		equivalent to attributes in
+		/sys/devices/system/node/nodeX/accessY/initiators/read_latency.
+		See Documentation/ABI/stable/sysfs-devices-node. The generic
+		target latency that is part of the whole path calculation is
+		the best performance latency provided by the HMAT SSLBIS table.
+
+
+What:		/sys/bus/cxl/devices/regionZ/write_latency
+Date:		Jan, 2024
+KernelVersion:	v6.9
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The write latency of the region. The number is
+		the worst write latency of all CXL memory devices that
+		contributes to the region in nanoseconds. Should be
+		equivalent to attributes in
+		/sys/devices/system/node/nodeX/accessY/initiators/write_latency.
+		See Documentation/ABI/stable/sysfs-devices-node. The generic
+		target latency that is part of the whole path calculation is
+		the best performance latency provided by the HMAT SSLBIS table.
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 7f19b533c5ae..d28d24524d41 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -343,6 +343,25 @@  static ssize_t commit_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(commit);
 
+#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.attrib == 0)				\
+		return -ENOENT;				\
+								\
+	return sysfs_emit(buf, "%u\n", cxlr->coord.attrib);	\
+}								\
+static DEVICE_ATTR_RO(attrib)
+
+ACCESS_ATTR(read_bandwidth);
+ACCESS_ATTR(read_latency);
+ACCESS_ATTR(write_bandwidth);
+ACCESS_ATTR(write_latency);
+
 static umode_t cxl_region_visible(struct kobject *kobj, struct attribute *a,
 				  int n)
 {
@@ -355,6 +374,23 @@  static umode_t cxl_region_visible(struct kobject *kobj, struct attribute *a,
 	 */
 	if (a == &dev_attr_uuid.attr && cxlr->mode != CXL_DECODER_PMEM)
 		return 0444;
+
+	if (a == &dev_attr_read_latency.attr &&
+	    cxlr->coord.read_latency == 0)
+		return 0;
+
+	if (a == &dev_attr_write_latency.attr &&
+	    cxlr->coord.write_latency == 0)
+		return 0;
+
+	if (a == &dev_attr_read_bandwidth.attr &&
+	    cxlr->coord.read_bandwidth == 0)
+		return 0;
+
+	if (a == &dev_attr_write_bandwidth.attr &&
+	    cxlr->coord.write_bandwidth == 0)
+		return 0;
+
 	return a->mode;
 }
 
@@ -654,6 +690,10 @@  static struct attribute *cxl_region_attrs[] = {
 	&dev_attr_resource.attr,
 	&dev_attr_size.attr,
 	&dev_attr_mode.attr,
+	&dev_attr_read_bandwidth.attr,
+	&dev_attr_write_bandwidth.attr,
+	&dev_attr_read_latency.attr,
+	&dev_attr_write_latency.attr,
 	NULL,
 };