[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
- Re: [Qemu-devel] [PATCH RFC 02/16] vl: smp: add checks for maxcpus based topologies, (continued)
- [Qemu-devel] [PATCH RFC 01/16] vl: smp_parse: cleanups, Andrew Jones, 2016/06/10
- [Qemu-devel] [PATCH RFC 04/16] hw/core/machine: Introduce pre_init, Andrew Jones, 2016/06/10
- [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Andrew Jones, 2016/06/10
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Paolo Bonzini, 2016/06/13
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Andrew Jones, 2016/06/13
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Paolo Bonzini, 2016/06/14
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init,
Andrew Jones <=
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Paolo Bonzini, 2016/06/14
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Andrew Jones, 2016/06/14
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Paolo Bonzini, 2016/06/14
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, David Gibson, 2016/06/14
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, Andrew Jones, 2016/06/15
- Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init, David Gibson, 2016/06/14
[Qemu-devel] [PATCH RFC 07/16] qom/cpu: make nr-cores, nr-threads real properties, Andrew Jones, 2016/06/10