Re: [PATCH] base/node / acpi: Change 'node_hmem_attrs' to 'access_coordinates'

[Jonathan Cameron]

On Fri, 12 May 2023 08:58:14 -0700
Dan Williams <dan.j.williams@xxxxxxxxx> wrote:

> Jonathan Cameron wrote:
> > On Fri, 05 May 2023 14:34:46 -0700
> > Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
> >   
> > > Dan Williams suggested changing the struct 'node_hmem_attrs' to
> > > 'access_coordinates' [1]. The struct is a container of r/w-latency and
> > > r/w-bandwidth numbers. Moving forward, this container will also be used by
> > > CXL to store the performance characteristics of each link hop in
> > > the PCIE/CXL topology. So, where node_hmem_attrs is just the access
> > > parameters of a memory-node, access_coordinates applies more broadly
> > > to hardware topology characteristics.  
> > 
> > Not that it hugely matters, but why the term "coordinates"?
> > Looks like Dan used that term, but I've not come across it being applied
> > in this circumstances and it isn't a case of being immediately obvious
> > to me what it means.
> > 
> > If it is just another vague entry in kernel word soup then I don't really
> > mind the term, but nice to give some reasoning in patch description.  
> 
> The inspiration here was past discussions that have been had about
> potential API changes for userspace contending with multiple memory
> types. The observation was that seemed like an exercise in having the
> application identify "where" it falls on a spectrum of bandwidth and
> latency needs.
> 
> So it's a tuple of read/write-latency and read/write-bandwidth.
> "Coordinates" is not a perfect fit. Sometimes it is just conveying
> values in isolation not a "location" relative to other performance
> points, but in the end this data is used to identify the performance
> operation point of a given memory-node.

Works for me. Can we add that to the patch description for the historians?

Having read a load more of the code using it, it now feels natural to me.

Thanks,

Jonathan