Re: [Qemu-devel] [PATCH v2 05/24] numa: move source of default CPUs to NUMA node mapping into boards

[Igor Mammedov]

On Wed, 3 May 2017 12:13:21 -0300
Eduardo Habkost <address@hidden> wrote:

> On Wed, May 03, 2017 at 02:56:59PM +0200, Igor Mammedov wrote:
> [...]
> > diff --git a/numa.c b/numa.c
> > index 6fc2393..ab1661d 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -294,9 +294,10 @@ static void validate_numa_cpus(void)
> >      g_free(seen_cpus);
> >  }
> >  
> > -void parse_numa_opts(MachineClass *mc)
> > +void parse_numa_opts(MachineState *ms)
> >  {
> >      int i;
> > +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> >  
> >      for (i = 0; i < MAX_NODES; i++) {
> >          numa_info[i].node_cpu = bitmap_new(max_cpus);
> > @@ -378,14 +379,16 @@ void parse_numa_opts(MachineClass *mc)  
> 
> 
> Expanding diff context:
> 
> >          /* Historically VCPUs were assigned in round-robin order to NUMA
> >           * nodes. However it causes issues with guest not handling it nice
> >           * in case where cores/threads from a multicore CPU appear on
> >           * different nodes. So allow boards to override default 
> > distribution
> >           * rule grouping VCPUs by socket so that VCPUs from the same socket
> >           * would be on the same node.
> >           */  
> 
> The above comment looks obsolete, as we are removing the code inside
> parse_numa_opts() that deals with grouping VCPUs by socket.
> 
> (Can be fixed by a follow-up patch, if necessary.)
it looks like, I'll respin series.
So I'll drop it on respin.

Looking at setting default mapping more,
it should be possible to remove it from parse_numa_opts()
altogether after this series.
Enable predefined by machine default mapping won't need
cpu_index_to_instance_props() translation. We just need
to set 'has_node_id = true' in possible_cpus for all cpus.

But I'd like to do this cleanup on top of this series.

> 
> > +        if (!mc->cpu_index_to_instance_props) {
> > +            error_report("default CPUs to NUMA node mapping isn't 
> > supported");
> > +            exit(1);
> > +        }
> >          if (i == nb_numa_nodes) {
> >              for (i = 0; i < max_cpus; i++) {
> > -                unsigned node_id = i % nb_numa_nodes;
> > -                if (mc->cpu_index_to_socket_id) {
> > -                    node_id = mc->cpu_index_to_socket_id(i) % 
> > nb_numa_nodes;
> > -                }
> > +                CpuInstanceProperties props;
> > +                props = mc->cpu_index_to_instance_props(ms, i);
> >  
> > -                set_bit(i, numa_info[node_id].node_cpu);
> > +                set_bit(i, numa_info[props.node_id].node_cpu);
> >              }
> >          }
> >    
> [...]
>