RE: [PATCH v3 02/22] xen/common: Default function to get the distance between nodes

[Hirokazu Takahashi <taka@xxxxxxxxxxxxx>]

Hello,

Thanks for the comments.

> > --- /dev/null
> > +++ b/xen/common/numa-distance-map.c
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#include <xen/errno.h>
> > +#include <xen/init.h>
> > +#include <xen/nodemask.h>
> 
> Why would any of these be needed here?

Ok, I will remove them.
 
> > +#include <xen/numa.h>
> > +
> > +#define LOCAL_DISTANCE      10
> > +#define REMOTE_DISTANCE     20
> > +
> > +/*
> > + * Get the distance between node 'from' and node 'to'.
> > + */
> 
> In the description you say this is a fallback, but that doesn't become
> clear from this comment at all.
> 
> > +uint8_t numa_node_distance(unsigned int from, unsigned int to)
> 
> If both parameters are node IDs, then why aren't they of type nodeid_t?

Okay, I will use nodeid_t.

Originally, this function was in the Device Tree code where nodeid_t could not 
be used due to a header dependency issue (the ARM NUMA header defined nodeid_t 
only after including the DT NUMA header, making it unavailable for extern 
declarations there). 

Now that the function has been separated into common code, this restriction no 
longer applies.
 
> And then why is the return value a fixed width type? See ./CODING_STYLE.

Okay, I will change the return type to unsigned int.
The node distance values range from 0U to 0xFFU, so I initially used a fixed
width type as the x86 implementation does.

> > +{
> > +    if ( from != to )
> > +        return REMOTE_DISTANCE;
> > +    return LOCAL_DISTANCE;
> > +}
> 
> Nit (style): If you don't want to use the conditional operator (so the
> body would be a single line), please have a blank line ahead of the
> function's main return statement.

Okay.

Hirokazu Takahashi.