| Message ID | 20240206222951.1833098-11-dave.jiang@intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | cxl: Add support to report region access coordinates to numa nodes | expand |
On Tue, 6 Feb 2024 15:28:38 -0700 Dave Jiang <dave.jiang@intel.com> 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> A few comments inline. Mostly about reducing duplication. > --- > Documentation/ABI/testing/sysfs-bus-cxl | 60 +++++++++++ > drivers/cxl/core/region.c | 134 ++++++++++++++++++++++++ > 2 files changed, 194 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index fff2581b8033..5f8c26815399 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/accessY/read_bandwidth Up to you, but it's fairly common to have multiple what lines in these files and you could easily combine at least the read/write descriptions. > +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. It is identical data that > + should appear in > + /sys/devices/system/node/nodeX/accessY/initiators/read_bandwidth. > + See Documentation/ABI/stable/sysfs-devices-node. access0 provides > + the number to the closest initiator and access1 provides the > + number to the closest CPU. > + > + > +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data that > + should appear in > + /sys/devices/system/node/nodeX/accessY/initiators/write_bandwidth. > + See Documentation/ABI/stable/sysfs-devices-node. access0 provides > + the number to the closest initiator and access1 provides the > + number to the closest CPU. > + > + > +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data > + that should appear in > + /sys/devices/system/node/nodeX/accessY/initiators/read_latency. > + See Documentation/ABI/stable/sysfs-devices-node. access0 provides > + the number to the closest initiator and access1 provides the > + number to the closest CPU. > + > + > +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data > + that should appear in > + /sys/devices/system/node/nodeX/accessY/initiators/write_latency. > + See Documentation/ABI/stable/sysfs-devices-node. access0 provides > + the number to the closest initiator and access1 provides the > + number to the closest CPU. > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 0c2337b1fd41..6347dbc746f9 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -30,6 +30,138 @@ > > static struct cxl_region *to_cxl_region(struct device *dev); > > +#define __ACCESS0_ATTR_RO(_name) { \ > + .attr = { .name = __stringify(_name), .mode = 0444 }, \ > + .show = _name##_access0_show, \ > +} > + > +#define ACCESS0_DEVICE_ATTR_RO(_name) \ > + struct device_attribute dev_attr_access0_##_name = __ACCESS0_ATTR_RO(_name) > + > +#define ACCESS0_ATTR(attrib) \ > +static ssize_t attrib##_access0_show(struct device *dev, \ > + struct device_attribute *attr, \ > + char *buf) \ > +{ \ > + struct cxl_region *cxlr = to_cxl_region(dev); \ > + \ > + if (cxlr->coord[0].attrib == 0) \ > + return -ENOENT; \ > + \ > + return sysfs_emit(buf, "%u\n", cxlr->coord[0].attrib); \ > +} \ > +static ACCESS0_DEVICE_ATTR_RO(attrib) > + > +ACCESS0_ATTR(read_bandwidth); > +ACCESS0_ATTR(read_latency); > +ACCESS0_ATTR(write_bandwidth); > +ACCESS0_ATTR(write_latency); > + > +static struct attribute *access0_coordinate_attrs[] = { > + &dev_attr_access0_read_bandwidth.attr, > + &dev_attr_access0_write_bandwidth.attr, > + &dev_attr_access0_read_latency.attr, > + &dev_attr_access0_write_latency.attr, > + NULL, > +}; > + > +static umode_t cxl_region_access0_coordinate_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct cxl_region *cxlr = to_cxl_region(dev); > + > + if (a == &dev_attr_access0_read_latency.attr && > + cxlr->coord[ACCESS_COORDINATE_LOCAL].read_latency == 0) > + return 0; > + > + if (a == &dev_attr_access0_write_latency.attr && > + cxlr->coord[ACCESS_COORDINATE_LOCAL].write_latency == 0) > + return 0; > + > + if (a == &dev_attr_access0_read_bandwidth.attr && > + cxlr->coord[ACCESS_COORDINATE_LOCAL].read_bandwidth == 0) > + return 0; > + > + if (a == &dev_attr_access0_write_bandwidth.attr && > + cxlr->coord[ACCESS_COORDINATE_LOCAL].write_bandwidth == 0) > + return 0; > + > + return a->mode; > +} > + > +#define __ACCESS1_ATTR_RO(_name) { \ > + .attr = { .name = __stringify(_name), .mode = 0444 }, \ > + .show = _name##_access1_show, \ > +} Maybe define __ACCESSX_ATTR_RO(_name, _access) etc to avoid some duplicaton. > + > +#define ACCESS1_DEVICE_ATTR_RO(_name) \ > + struct device_attribute dev_attr_access1_##_name = __ACCESS1_ATTR_RO(_name) > + > +#define ACCESS1_ATTR(attrib) \ > +static ssize_t attrib##_access1_show(struct device *dev, \ > + struct device_attribute *attr, \ > + char *buf) \ > +{ \ > + struct cxl_region *cxlr = to_cxl_region(dev); \ > + \ > + if (cxlr->coord[1].attrib == 0) \ > + return -ENOENT; \ > + \ > + return sysfs_emit(buf, "%u\n", cxlr->coord[1].attrib); \ > +} \ > +static ACCESS1_DEVICE_ATTR_RO(attrib) > + > +ACCESS1_ATTR(read_bandwidth); > +ACCESS1_ATTR(read_latency); > +ACCESS1_ATTR(write_bandwidth); > +ACCESS1_ATTR(write_latency); > + > +static struct attribute *access1_coordinate_attrs[] = { > + &dev_attr_access1_read_bandwidth.attr, > + &dev_attr_access1_write_bandwidth.attr, > + &dev_attr_access1_read_latency.attr, > + &dev_attr_access1_write_latency.attr, > + NULL, > +}; > + > +static umode_t cxl_region_access1_coordinate_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct cxl_region *cxlr = to_cxl_region(dev); > + > + if (a == &dev_attr_access1_read_latency.attr && > + cxlr->coord[ACCESS_COORDINATE_CPU].read_latency == 0) > + return 0; > + > + if (a == &dev_attr_access1_write_latency.attr && > + cxlr->coord[ACCESS_COORDINATE_CPU].write_latency == 0) > + return 0; > + > + if (a == &dev_attr_access1_read_bandwidth.attr && > + cxlr->coord[ACCESS_COORDINATE_CPU].read_bandwidth == 0) > + return 0; > + > + if (a == &dev_attr_access1_write_bandwidth.attr && > + cxlr->coord[ACCESS_COORDINATE_CPU].write_bandwidth == 0) > + return 0; > + > + return a->mode; > +} > + > +static const struct attribute_group cxl_region_access0_coordinate_group = { > + .name = "access0", > + .attrs = access0_coordinate_attrs, > + .is_visible = cxl_region_access0_coordinate_visible, > +}; > + > +static const struct attribute_group cxl_region_access1_coordinate_group = { > + .name = "access1", > + .attrs = access1_coordinate_attrs, > + .is_visible = cxl_region_access1_coordinate_visible, > +}; > + > static ssize_t uuid_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > @@ -2039,6 +2171,8 @@ static const struct attribute_group *region_groups[] = { > &cxl_base_attribute_group, > &cxl_region_group, > &cxl_region_target_group, > + &cxl_region_access0_coordinate_group, > + &cxl_region_access1_coordinate_group, > NULL, > }; >
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl index fff2581b8033..5f8c26815399 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/accessY/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. It is identical data that + should appear in + /sys/devices/system/node/nodeX/accessY/initiators/read_bandwidth. + See Documentation/ABI/stable/sysfs-devices-node. access0 provides + the number to the closest initiator and access1 provides the + number to the closest CPU. + + +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data that + should appear in + /sys/devices/system/node/nodeX/accessY/initiators/write_bandwidth. + See Documentation/ABI/stable/sysfs-devices-node. access0 provides + the number to the closest initiator and access1 provides the + number to the closest CPU. + + +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data + that should appear in + /sys/devices/system/node/nodeX/accessY/initiators/read_latency. + See Documentation/ABI/stable/sysfs-devices-node. access0 provides + the number to the closest initiator and access1 provides the + number to the closest CPU. + + +What: /sys/bus/cxl/devices/regionZ/accessY/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. It is identical data + that should appear in + /sys/devices/system/node/nodeX/accessY/initiators/write_latency. + See Documentation/ABI/stable/sysfs-devices-node. access0 provides + the number to the closest initiator and access1 provides the + number to the closest CPU. diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 0c2337b1fd41..6347dbc746f9 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -30,6 +30,138 @@ static struct cxl_region *to_cxl_region(struct device *dev); +#define __ACCESS0_ATTR_RO(_name) { \ + .attr = { .name = __stringify(_name), .mode = 0444 }, \ + .show = _name##_access0_show, \ +} + +#define ACCESS0_DEVICE_ATTR_RO(_name) \ + struct device_attribute dev_attr_access0_##_name = __ACCESS0_ATTR_RO(_name) + +#define ACCESS0_ATTR(attrib) \ +static ssize_t attrib##_access0_show(struct device *dev, \ + struct device_attribute *attr, \ + char *buf) \ +{ \ + struct cxl_region *cxlr = to_cxl_region(dev); \ + \ + if (cxlr->coord[0].attrib == 0) \ + return -ENOENT; \ + \ + return sysfs_emit(buf, "%u\n", cxlr->coord[0].attrib); \ +} \ +static ACCESS0_DEVICE_ATTR_RO(attrib) + +ACCESS0_ATTR(read_bandwidth); +ACCESS0_ATTR(read_latency); +ACCESS0_ATTR(write_bandwidth); +ACCESS0_ATTR(write_latency); + +static struct attribute *access0_coordinate_attrs[] = { + &dev_attr_access0_read_bandwidth.attr, + &dev_attr_access0_write_bandwidth.attr, + &dev_attr_access0_read_latency.attr, + &dev_attr_access0_write_latency.attr, + NULL, +}; + +static umode_t cxl_region_access0_coordinate_visible(struct kobject *kobj, + struct attribute *a, int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct cxl_region *cxlr = to_cxl_region(dev); + + if (a == &dev_attr_access0_read_latency.attr && + cxlr->coord[ACCESS_COORDINATE_LOCAL].read_latency == 0) + return 0; + + if (a == &dev_attr_access0_write_latency.attr && + cxlr->coord[ACCESS_COORDINATE_LOCAL].write_latency == 0) + return 0; + + if (a == &dev_attr_access0_read_bandwidth.attr && + cxlr->coord[ACCESS_COORDINATE_LOCAL].read_bandwidth == 0) + return 0; + + if (a == &dev_attr_access0_write_bandwidth.attr && + cxlr->coord[ACCESS_COORDINATE_LOCAL].write_bandwidth == 0) + return 0; + + return a->mode; +} + +#define __ACCESS1_ATTR_RO(_name) { \ + .attr = { .name = __stringify(_name), .mode = 0444 }, \ + .show = _name##_access1_show, \ +} + +#define ACCESS1_DEVICE_ATTR_RO(_name) \ + struct device_attribute dev_attr_access1_##_name = __ACCESS1_ATTR_RO(_name) + +#define ACCESS1_ATTR(attrib) \ +static ssize_t attrib##_access1_show(struct device *dev, \ + struct device_attribute *attr, \ + char *buf) \ +{ \ + struct cxl_region *cxlr = to_cxl_region(dev); \ + \ + if (cxlr->coord[1].attrib == 0) \ + return -ENOENT; \ + \ + return sysfs_emit(buf, "%u\n", cxlr->coord[1].attrib); \ +} \ +static ACCESS1_DEVICE_ATTR_RO(attrib) + +ACCESS1_ATTR(read_bandwidth); +ACCESS1_ATTR(read_latency); +ACCESS1_ATTR(write_bandwidth); +ACCESS1_ATTR(write_latency); + +static struct attribute *access1_coordinate_attrs[] = { + &dev_attr_access1_read_bandwidth.attr, + &dev_attr_access1_write_bandwidth.attr, + &dev_attr_access1_read_latency.attr, + &dev_attr_access1_write_latency.attr, + NULL, +}; + +static umode_t cxl_region_access1_coordinate_visible(struct kobject *kobj, + struct attribute *a, int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct cxl_region *cxlr = to_cxl_region(dev); + + if (a == &dev_attr_access1_read_latency.attr && + cxlr->coord[ACCESS_COORDINATE_CPU].read_latency == 0) + return 0; + + if (a == &dev_attr_access1_write_latency.attr && + cxlr->coord[ACCESS_COORDINATE_CPU].write_latency == 0) + return 0; + + if (a == &dev_attr_access1_read_bandwidth.attr && + cxlr->coord[ACCESS_COORDINATE_CPU].read_bandwidth == 0) + return 0; + + if (a == &dev_attr_access1_write_bandwidth.attr && + cxlr->coord[ACCESS_COORDINATE_CPU].write_bandwidth == 0) + return 0; + + return a->mode; +} + +static const struct attribute_group cxl_region_access0_coordinate_group = { + .name = "access0", + .attrs = access0_coordinate_attrs, + .is_visible = cxl_region_access0_coordinate_visible, +}; + +static const struct attribute_group cxl_region_access1_coordinate_group = { + .name = "access1", + .attrs = access1_coordinate_attrs, + .is_visible = cxl_region_access1_coordinate_visible, +}; + static ssize_t uuid_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -2039,6 +2171,8 @@ static const struct attribute_group *region_groups[] = { &cxl_base_attribute_group, &cxl_region_group, &cxl_region_target_group, + &cxl_region_access0_coordinate_group, + &cxl_region_access1_coordinate_group, NULL, };
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> --- Documentation/ABI/testing/sysfs-bus-cxl | 60 +++++++++++ drivers/cxl/core/region.c | 134 ++++++++++++++++++++++++ 2 files changed, 194 insertions(+)