Re: [Qemu-devel] [PATCH v7] Allow setting NUMA distance for different NUMA nodes

[Igor Mammedov]

On Mon, 24 Apr 2017 16:52:48 +0800
He Chen <address@hidden> wrote:

> On Fri, Apr 21, 2017 at 11:53:01AM +0200, Igor Mammedov wrote:
> > On Fri, 21 Apr 2017 15:32:15 +0800
> > He Chen <address@hidden> wrote:
> >   
> ...
> > > +static void validate_numa_distance(void)
> > > +{
> > > +    int src, dst;
> > > +    bool is_asymmetrical = false;
> > > +
> > > +    for (src = 0; src < nb_numa_nodes; src++) {
> > > +        for (dst = 0; dst < nb_numa_nodes; dst++) {  
> >                 ^^^ checks inside this loop are symmetric,
> >                 is there any reason it wouldn't work wit previous variant 
> > 'dst = src'?
> >   
> I am sorry I don't have a clear understanding about what you suggested
> here. You mean we should check whether the table is symmetric in this
> loop?
> Regarding 'dst = src', it represents local distance, user would
> omit setting it and we will fix it in complete_init_numa_distance. Did I
> mistake something? Could you please explain in more detail? Thanks.
I was trying to say that
since all checks inside this loop are symmetric
you can scan only half of matrix, i.e.:
  ...
     for (dst = src; dst < nb_numa_nodes; dst++) {
  ...
but I won't insist on it if you prefer leave it as is.

> > > +            if (numa_info[src].present && numa_info[dst].present) {  
> > we don't support sparse nodes, so this condition is always true
> > and not needed as earlier code assures that all nodes upto nb_numa_nodes
> > are present, greep for "numa: Node ID missing: %d"
> > so you can remove this check in this func and in 
> > complete_init_numa_distance()
> >   
> > > +                if (numa_info[src].distance[dst] == 0 &&
> > > +                    numa_info[dst].distance[src] == 0) {
> > > +                    if (src != dst) {
> > > +                        error_report("The distance between node %d and 
> > > %d is missing, "
> > > +                                     "please provide all unique node 
> > > pair distances.",
> > > +                                     src, dst);  
> > s/all unique node .../ at least one distance value between each nodes 
> > should be provided/
> > 
> > or something like this
> >   
> > > +                        exit(EXIT_FAILURE);
> > > +                    }
> > > +                }
> > > +
> > > +                if (((numa_info[src].distance[dst] != 0) &&
> > > +                    (numa_info[dst].distance[src] != 0)) &&
> > > +                    (numa_info[src].distance[dst] !=
> > > +                    numa_info[dst].distance[src])) {
> > > +                    is_asymmetrical = true;
> > > +                }
> > > +            }
> > > +        }
> > > +    }
> > > +
[...]