qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Andrew Jones
Subject: Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init
Date: Tue, 14 Jun 2016 13:39:04 +0200
User-agent: Mutt/1.5.23.1 (2014-03-12)

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:
> >>> +    if (sockets == -1 || cores == -1 || threads == -1 ||
> >>> +        maxcpus == -1 || cpus == -1) {
> >>> +        error_report("cpu topology: "
> >>> +                     "all machine properties must be specified");
> >>> +        exit(1);
> >>> +    }
> >>> +
> >>
> >> I think it's sane to accept some defaults.  It must not be the DWIM
> >> thing that -smp does (which is targeted to Windows's dislike of
> >> multi-socket machine on consumer hardware).  It must be something that
> >> makes sense, and my proposal is:
> >>
> >> - threads: 1
> >> - cores: 1
> >> - sockets:
> >>   - maxcpus / (cores * threads) if maxcpus given
> >>   - cpus / (cores * threads) if cpus given
> >>   - else 1
> >> - maxcpus: cores * threads * sockets
> >> - cpus: maxcpus
> > 
> > I think some machines may prefer
> > 
> > - threads: 1
> > - sockets: 1
> > - cores:
> >   - maxcpus / (sockets * threads) if maxcpus given
> >   - cpus / (sockets * threads) if cpus given
> >   - else 1
> 
> smp_cores is only used by pseries and x86 machines.  I expect machines
> that must be single-socket to disregard smp_sockets altogether.

mach-virt currently assumes 1 "socket" with N cores when -smp N is used.
This is because, until recently, ARM machines didn't define sockets. I
presume the mindset will remain that cpus=N means cores=N even after
introducing support for multi-socket topology.

> 
> >> - any argument < 1
> > 
> > What if a user says threads=0, because they don't have HT, and assume
> > that a value of zero will get them a non-HT system? Or what about
> > machines that don't describe sockets, so a user inputs zero? In both
> > those cases I agree we should just use 1, but from a user's perspective,
> > it might seem weird.
> 
> 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.

> 
> >> - any argument > some compile-time value (1024?) to avoid overflows
> > 
> > Agreed. We should do this regardless of this series.
> > 
> >> - 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. 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

> 
> >> - cpus > sockets * cores * threads
> >> - maxcpus != cores * threads * sockets
> > 
> > We check these two (the 2nd added with this series) already.
> 
> Yup, I was just making a complete list.
> 
> >> Alone the last relation shows that requiring all four of maxcpus, cores,
> >> threads and sockets is unnecessary. :)
> > 
> > Not really. It depends on if you assume sockets, cores or threads to
> > be the N in maxcpus=N. We could just require sockets, cores, and threads
> > to be input, which allows us to easily calculate maxcpus, and then set
> > cpus from that. In that case maxcpus would just be an available input
> > for sanity checking.
> 
> Or 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

> 
> >> -smp should do its legacy magic and assign all five values based on it.
> >> If the results do not match the obvious s/smp/-machine/ command line it
> >> should warn, and perhaps suggest the equivalent -machine command line.
> >> It doesn't have to be a minimal command line, just equivalent.
> > 
> > This is a good idea. I'm just still not sure what the -machine command
> > line should/should not assume.
> 
> It's okay.  Adding defaults can be done later, as long as strict checks
> are in place from the beginning and there is a clean separation between
> -smp defaults and -machine.
> 
> Relaxing checks can be done later, so I am much more interested in
> having strict checks than in having defaults.

Yes, my current thinking (which is always adjustable :-) is to start
with strict checks, and maybe always have them in this common code.
Machines that override this can implement defaults based on their
machine-specific knowledge, allowing the user to get sane topologies
without really needing to know what they want.

> 
> 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...

Thanks,
drew



reply via email to

[Prev in Thread] Current Thread [Next in Thread]