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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

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