RE: [XEN RFC PATCH 24/40] xen/arm: introduce a helper to parse device tree NUMA distance map

[Wei Chen <Wei.Chen@arm.com>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2021年9月2日 14:00
> To: Stefano Stabellini <sstabellini@kernel.org>; Wei Chen
> <Wei.Chen@arm.com>
> Cc: xen-devel@lists.xenproject.org; julien@xen.org; Bertrand Marquis
> <Bertrand.Marquis@arm.com>
> Subject: Re: [XEN RFC PATCH 24/40] xen/arm: introduce a helper to parse
> device tree NUMA distance map
> 
> On 01.09.2021 18:21, Stefano Stabellini wrote:
> > On Wed, 1 Sep 2021, Wei Chen wrote:
> >>> Stefano Stabellini
> >>> Sent: 2021年9月1日 5:36
> >>>
> >>> On Tue, 31 Aug 2021, Wei Chen wrote:
> >>>> I don't really like this implementation. I want the behavior of
> >>>> numa_set_distance just like the function name, do not include
> >>>> implicit operations. Otherwise, except the user read this function
> >>>> implementation before he use it, he probably doesn't know this
> >>>> function has done so many things.
> >>>
> >>> You can leave numa_set_distance as-is without any implicit operations.
> >>>
> >>> In that case, just call numa_set_distance twice from numa_set_distance
> >>> for both from/to and to/from. numa_set_distance could return error is
> >>
> >> I am OK for the first sentence. But...
> >>
> >>> the entry was already set to a different value or success otherwise
> >>> (also in the case it was already set to the same value). This would
> >>
> >> ... I prefer not to check the previous value. Subsequent
> numa_set_distance
> >> call will override previous calls. Keep numa_set_distance as simple as
> >> it can. And when you pass new data to numa_set_distance, it doesn't
> >> know whether the previous data was correct or the new data is correct.
> >> Only caller may have known.
> >
> > That might be OK but if not numa_set_distance then somebody else needs
> > to check against overwriting previous values. That is to be able to spot
> > bad device tree cases like:
> >
> >   0 1 20
> >   1 0 40
> 
> What's wrong with this? At least the ACPI spec cares to specifically
> permit this case:
> 

This is because some notes description in device tree NUMA binding:
- distance-matrix
  This property defines a matrix to describe the relative distances
  between all numa nodes.
  It is represented as a list of node pairs and their relative distance.

  Note:
	1. Each entry represents distance from first node to second node.
	The distances are equal in either direction.
https://www.kernel.org/doc/Documentation/devicetree/bindings/numa.txt

So we treat this case is wrong in DT. But as you said ACPI allow such
case, that means real machines allow different distances for one pair
nodes in different directions.

> "Except for the relative distance from a System Locality to itself,
>  each relative distance is stored twice in the matrix. This provides
>  the capability to describe the scenario where the relative distances
>  for the two directions between System Localities is different."
> 
> Jan