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

[Jan Beulich <jbeulich@xxxxxxxx>]

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:

"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