smp_parse has a couple problems. First, it should use max_cpus, not smp_cpus when calculating missing topology information. Conversely, if maxcpus is not input, then the topology should dictate max_cpus, as the topology may support more than the input smp_cpus number. Second, smp_parse shouldn't silently adjust the number of threads a user provides, which it currently does in order to ensure the topology supports the number of cpus provided. smp_parse should rather complain and exit when input isn't consistent. This patch fixes those issues and attempts to clarify the code a bit too.
I don't believe we need to consider compatibility with the old behaviors. Specifying something like -smp 4,sockets=1,cores=1,threads=1 is wrong, even though it currently works (the number of threads is silently adjusted to 4). And, specifying something like -smp 4,sockets=1,cores=4,threads=1,maxcpus=8 is also wrong, as there's no way to hotplug the additional 4 cpus with this topology. So, any users doing these things should be corrected. The new error message this patch provides should help them do that. Below are some examples with before/after results // topology should support up to max_cpus < -smp 4,sockets=2,maxcpus=8 sockets=2 cores=2 threads=1 smp_cpus=4 max_cpus=8 > -smp 4,sockets=2,maxcpus=8 sockets=2 cores=4 threads=1 > smp_cpus=4 max_cpus=8 // topology supports more than smp_cpus, max_cpus should be higher < -smp 4,sockets=4,cores=2 sockets=4 cores=2 threads=1 smp_cpus=4 max_cpus=4 > -smp 4,sockets=4,cores=2 sockets=4 cores=2 threads=1 > smp_cpus=4 max_cpus=8 // shouldn't silently adjust threads < -smp 4,sockets=1,cores=2,threads=1 sockets=1 cores=2 threads=2 smp_cpus=4 max_cpus=4 > -smp 4,sockets=1,cores=2,threads=1 "maxcpus must be equal to or > greater than smp" < -smp 4,sockets=2,cores=2,threads=4,maxcpus=8 sockets=2 cores=2 threads=1 smp_cpus=4 max_cpus=8 > -smp 4,sockets=2,cores=2,threads=4,maxcpus=8 "cpu topology: sockets (2) * > cores (2) * threads (4) != max_cpus (8)" Signed-off-by: Andrew Jones <[email protected]> --- vl.c | 59 ++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/vl.c b/vl.c index f4a6e5e05bce2..23b21a5fbca50 100644 --- a/vl.c +++ b/vl.c @@ -1270,35 +1270,52 @@ static QemuOptsList qemu_smp_opts = { static void smp_parse(QemuOpts *opts) { if (opts) { - - unsigned cpus = qemu_opt_get_number(opts, "cpus", 0); unsigned sockets = qemu_opt_get_number(opts, "sockets", 0); unsigned cores = qemu_opt_get_number(opts, "cores", 0); unsigned threads = qemu_opt_get_number(opts, "threads", 0); + unsigned cpus; + + smp_cpus = qemu_opt_get_number(opts, "cpus", 0); + max_cpus = qemu_opt_get_number(opts, "maxcpus", 0); + + cpus = max_cpus ? max_cpus : smp_cpus; /* compute missing values, prefer sockets over cores over threads */ - if (cpus == 0 || sockets == 0) { - sockets = sockets > 0 ? sockets : 1; - cores = cores > 0 ? cores : 1; - threads = threads > 0 ? threads : 1; - if (cpus == 0) { - cpus = cores * threads * sockets; - } - } else { - if (cores == 0) { - threads = threads > 0 ? threads : 1; - cores = cpus / (sockets * threads); - } else { - threads = cpus / (cores * sockets); - } + if (cpus == 0) { + cpus = sockets ? sockets : 1; + cpus = cpus * cores ? cpus * cores : cpus; + cpus = cpus * threads ? cpus * threads : cpus; } - max_cpus = qemu_opt_get_number(opts, "maxcpus", 0); + if (sockets == 0) { + sockets = 1; + } - smp_cpus = cpus; - smp_cores = cores > 0 ? cores : 1; - smp_threads = threads > 0 ? threads : 1; + if (cores == 0) { + threads = threads ? threads : 1; + cores = cpus / (sockets * threads); + cores = cores ? cores : 1; + } + + if (threads == 0) { + threads = cpus / (sockets * cores); + threads = threads ? threads : 1; + } + + if (max_cpus == 0) { + max_cpus = sockets * cores * threads; + } + if (sockets * cores * threads != max_cpus) { + fprintf(stderr, "cpu topology: " + "sockets (%u) * cores (%u) * threads (%u) != max_cpus (%u)\n", + sockets, cores, threads, max_cpus); + exit(1); + } + + smp_cpus = smp_cpus ? smp_cpus : max_cpus; + smp_cores = cores; + smp_threads = threads; } if (max_cpus == 0) { @@ -1309,11 +1326,11 @@ static void smp_parse(QemuOpts *opts) fprintf(stderr, "Unsupported number of maxcpus\n"); exit(1); } + if (max_cpus < smp_cpus) { fprintf(stderr, "maxcpus must be equal to or greater than smp\n"); exit(1); } - } static void realtime_init(void) -- 1.9.3
