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

[He Chen]

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.
> > +            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;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> > +    if (is_asymmetrical) {
> > +        for (src = 0; src < nb_numa_nodes; src++) {
> > +            for (dst = 0; dst < nb_numa_nodes; dst++) {
> > +                if (numa_info[src].present && numa_info[dst].present) {
> > +                    if ((src != dst) && (numa_info[src].distance[dst] == 
> > 0)) {
> > +                        error_report("At least one asymmetrical pair of "
> > +                                "distances is given, please provide 
> > distances "
> > +                                "for both directions of all node pairs.");
> > +                        exit(EXIT_FAILURE);
> > +                    }
> > +                }
> > +            }
> > +        }
> > +    }
> > +}
> > +
> > +static void complete_init_numa_distance(void)
> > +{
> > +    int src, dst;
> > +
> > +    /* fixup NUMA distance by symmetric policy because if it is an
> > +     * asymmtric distance table, it should be a complete table and there
> > +     * would not be any missing distance except local node, which is
> > +     * verified by validate_numa_distance above.
> > +     */
> > +    for (src = 0; src < nb_numa_nodes; src++) {
> > +        for (dst = 0; dst < nb_numa_nodes; dst++) {
> > +            if (numa_info[src].present && numa_info[dst].present) {
> > +                if (numa_info[src].distance[dst] == 0) {
> > +                    if (src == dst) {
> > +                        numa_info[src].distance[dst] = NUMA_DISTANCE_MIN;
> > +                    } else {
> > +                        numa_info[src].distance[dst] = 
> > numa_info[dst].distance[src];
> > +                    }
> > +                }
> > +            }
> > +        }
> > +    }
> > +}
...