Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init

[Andrew Jones <drjones@redhat.com>]

On Tue, Jun 14, 2016 at 01:53:05PM +0200, Paolo Bonzini wrote:
> 
> 
> On 14/06/2016 13:39, Andrew Jones wrote:
> > On Tue, Jun 14, 2016 at 10:17:49AM +0200, Paolo Bonzini wrote:
> >> On 13/06/2016 22:35, Andrew Jones wrote:
> >>> On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:
> >>>> On 10/06/2016 19:40, Andrew Jones wrote:
> >> They should just not specify it and get a default of 1. ;)
> > 
> > Yeah, threads, the only one we should never calculate, could be
> > optional. If not specified, defaulting to 1 makes perfect sense.
> > But, threads=0, which is weird, but in a way specifying that it's
> > not specified, also makes some sense.
> 
> If it's weird, let's make it invalid. :)

I'm weird, and starting to like it :-)

> 
> >>>> - cpus % (cores * threads) != 0
> >>>
> >>> Hmm. This makes sense where cpus is the number of cpu packages,
> >>
> >> I'm not sure I understand what you mean here.  The point is that the
> >> machine starts with an integral number of sockets.
> > 
> > OK, s/cpus/maxcpus/ then. By using the currently online number, I
> > thought you were starting to prepare for cpu packages, which are
> > indivisible sets of cores and threads.
> 
> Yes, that's what I meant to do.  Isn't cpus what you call
> "total-online-threads" below?

It is. I was thinking it may need to be redefined if we wanted to
hotplug these "indivisible sets of cores and threads" (sockets),
instead of what we do today, which is to hotplug one "cpu" at a time.
However, I just chatted with Igor, and he says cpu hotplug operates
on logical processors, and thus it's fine to talk about hotplugging
threads. Even real hardware does this. Real hardware will plug a
socket full of threads, but then the firmware may keep most of them
disabled until they're "hotplugged". So, I think the value of cpus
can be anything. Even the useless value of zero.

> 
> > But now that I think about
> > it, cpus % (cores * threads) isn't right for that either. It should
> > be total-online-threads % (cores * threads) != 0
> > 
> >> [...] you could just specify -machine cpus=16,maxcpus=32 and expect 1
> >> core/socket, 1 thread/core, and 16 to 32 sockets.
> > 
> > Or 32 cores/socket (only 16 populated), 1 thread/core
> 
> Does that make sense?  How do you "populate" parts of a socket lazily? I
> know it's all virtual, but... ;)

In real hardware that would be a socket of 32 cores, 16 disabled.
Hotplug can then enable them one at a time.

> 
> >> Though I think that we can at least agree on defaults for threads,
> >> maxcpus and cpus.  The only sticky point is sockets vs. cores.  We can
> >> make them both mandatory for now.  That is: cores and sockets are
> >> mandatory until we decide which one to default to 1; threads is
> >> optional; cpus is mandatory if you want hotplug, otherwise it's
> >> redundant; maxcpus is optional and always redundant.
> > 
> > Agreed. I'll do the following for the next round of this series
> > 
> >  threads: 1
> >  cores:   required
> >  sockets: required
> >  maxcpus: maxcpus ? maxcpus : sockets * cores * threads
> >  cpus:    cpus ? cpus : maxcpus
> > 
> > If maxcpus is input, it's redundant, but should be sanity checked.
> > Maybe the user wants to use the QEMU cmdline to check their math...
> 
> Yes, all this is fine.  As to the checks, here's my list:
> 
> >>> - any argument < 1
> >>> - any argument > some compile-time value (1024?) to avoid overflows
> >>> - cpus % (cores * threads) != 0
> >>> - cpus > sockets * cores * threads
> >>> - maxcpus != cores * threads * sockets
> 
> ?  The second, fourth and fifth are fine, and IMO the first too.  What
> about the "integral package" check?   It needs to be disabled with some
> ugly global when using -smp, but apart from that I believe it's better
> to have it.

I'd keep =0 allowed. In most cases it just means =1, same as if the
property wasn't input. In some cases, cpus,maxcpus it creates a
useless machine, but maybe could accelerate QEMU runs for the purpose
of probing? Eh, it probably adds more complication than necessary...

Considering cpus should mean logical processors, I think that can
have any value on (0, maxcpus]

Thanks,
drew