Re: [PATCH v3 07/22] xen/device-tree: Read NUMA node distance from Device Tree 'distance-map'

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.06.2026 01:36, Hirokazu Takahashi wrote:
>>> +    entry_count = len / sizeof(__be32);
>>
>> Nit: Better sizeof(<expression>).
> 
> Is the following line better?
> entry_count = len / sizeof(*matrix);

Yes.

>>>  /*
>>>   * Get the distance between node 'from' and node 'to'.
>>>   */
>>>  uint8_t numa_node_distance(unsigned int from, unsigned int to)
>>>  {
>>> -    if ( from != to )
>>> -        return REMOTE_DISTANCE;
>>> -    return LOCAL_DISTANCE;
>>
>> Why did you introduce the function as a fallback when now you remove the
>> fallback logic entirely? Can't you introduce the function right here,
>> omitting the earlier patch?
> 
> I will remove the earlier patch.
> 
>>> +    const unsigned int nr_nodes = last_node(node_online_map) + 1U;
>>> +
>>> +    if ( from >= nr_nodes || to >= nr_nodes )
>>> +        return from == to ? LOCAL_DISTANCE : REMOTE_DISTANCE;
>>
>> What if either node is NUMA_NO_NODE?
> 
> This behavior comes from the Linux kernel. It seems it exists as a defensive
> fallback to keep the system running even with invalid or unassigned nodes.
> 
> Do you think it is better to make it return 0xFF instead whenever any
> out-of-bounds node or NUMA_NO_NODE is passed?

Whether it's 0xff or REMOTE_DISTANCE I'm not quite sure. But it certainly
shouldn't be LOCAL_DISTANCE.

>>> +    for ( i = 0U; i < nr_nodes; i++ )
>>> +        for ( j = 0U; j < nr_nodes; j++ )
>>
>> Why the U suffixes?
> 
> I added the U suffixes because variables i and j are unsigned types. 
> If a plain 0 is preferred here, I will remove them.

They, even if only slightly, hamper readability. We tend to add them only
when there actually is a need.

Jan