Re: [Qemu-ppc] [RFC 1/4] numa, spapr: add thread-id in the possible_cpus list

[Laurent Vivier]

On 13/02/2019 09:42, Igor Mammedov wrote:
> On Wed, 13 Feb 2019 12:25:45 +1100
> David Gibson <address@hidden> wrote:
>
> > On Tue, Feb 12, 2019 at 10:48:24PM +0100, Laurent Vivier wrote:
> > > spapr_possible_cpu_arch_ids() counts only cores, and so
> > > the number of available CPUs is the number of vCPU divided
> > > by smp_threads.
> > >
> > > ... -smp 4,maxcpus=8,cores=2,threads=2,sockets=2 -numa node,cpus=0,cpus=1 \
> > >                                                   -numa node,cpus=3,cpus=4 \
> > >                                                   -numa node -numa node
> > >
> > > This generates (info hotpluggable-cpus)
> > >
> > >    node-id: 0 core-id: 0 thread-id: 0 [thread-id: 1]
> > >    node-id: 0 core-id: 6 thread-id: 0 [thread-id: 1]
> > >    node-id: 1 core-id: 2 thread-id: 0 [thread-id: 1]
> > >    node-id: 1 core-id: 4 thread-id: 0 [thread-id: 1]
> > >
> > > And this command line generates the following error:
> > >
> > >    CPU(s) not present in any NUMA nodes: CPU 3 [core-id: 6]
> > >
> > > That is wrong because CPU 3 [core-id: 6] is assigned to node-id 0
> > > Moreover "cpus=4" is not valid, because it means core-id 8 but
> > > maxcpus is 8.
> > >
> > > With this patch we have now:
> > >
> > >    node-id: 0 core-id: 0 thread-id: 0
> > >    node-id: 0 core-id: 0 thread-id: 1
> > >    node-id: 0 core-id: 1 thread-id: 0
> > >    node-id: 1 core-id: 1 thread-id: 1
> > >    node-id: 0 core-id: 2 thread-id: 1
> > >    node-id: 1 core-id: 2 thread-id: 0
> > >    node-id: 0 core-id: 3 thread-id: 1
> > >    node-id: 0 core-id: 3 thread-id: 0
> >
> > I'm afraid this is not the right solution.  The point of the
> > hotpluggable cpus table is that it has exactly one entry for each
> > hotpluggable unit.  For PAPR that's a core, not a thread.
> >
> > So, the problem is with how the NUMA configuration code is
> > interpreting possible-cpus, not how the machine is building the table.
>
> I'd suggest to deprecate/remove 'cpus' suboption in -numa.
> One should use '-numa cpu' instead, which is written with
> possible_cpus in mind.

I agree.

Should I keep the patch to remove the incomplete CPU mapping support?

Thanks,
Laurent