[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 02/16] vl: smp: add checks for maxcpus based
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH RFC 02/16] vl: smp: add checks for maxcpus based topologies |
Date: |
Tue, 14 Jun 2016 11:28:52 +1000 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Fri, Jun 10, 2016 at 07:40:13PM +0200, Andrew Jones wrote:
> smp_parse computes missing smp options. Unfortunately cores and
> threads are computed by dividing smp_cpus, instead of max_cpus.
> This is incorrect because the topology doesn't leave room for
> hotplug. More unfortunately, we can't change it easily, as doing
> so would impact existing command lines. This patch adds a warning
> when the topology doesn't add up, and then checks that the topology
> at least computes when sockets are recalculated. If not, then it
> does fail.
>
> Adding the new failure is justified by the fact that we don't
> store the number of input sockets, and thus all consumers of
> cpu topology information recalculate it. If they choose to
> (correctly) calculate it based on maxcpus, then we need to
> guard them against building topologies which provide more cpu
> slots than are the maximum allowed cpus.
>
> Signed-off-by: Andrew Jones <address@hidden>
Hmm.. this makes sense to me. Except that I've never been clear if
sockets= was supposed to match initial cpus or maxcpus.
> ---
> vl.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index 7b96e787922f9..8d482cb1bf020 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1227,6 +1227,7 @@ static void smp_parse(QemuOpts *opts)
> 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);
> + bool sockets_input = sockets > 0;
>
> /* compute missing values, prefer sockets over cores over threads */
> if (cpus == 0 || sockets == 0) {
> @@ -1269,6 +1270,24 @@ static void smp_parse(QemuOpts *opts)
> exit(1);
> }
>
> + if (sockets_input && sockets * cores * threads != max_cpus) {
> + unsigned sockets_rounded = DIV_ROUND_UP(max_cpus, cores *
> threads);
> +
> + error_report("warning: cpu topology: "
> + "sockets (%u) * cores (%u) * threads (%u) != "
> + "maxcpus (%u). Trying sockets=%u.",
> + sockets, cores, threads, max_cpus, sockets_rounded);
> + sockets = sockets_rounded;
> +
> + if (sockets * cores * threads > max_cpus) {
> + error_report("cpu topology: "
> + "sockets (%u) * cores (%u) * threads (%u) > "
> + "maxcpus (%u)",
> + sockets, cores, threads, max_cpus);
> + exit(1);
> + }
> + }
> +
> smp_cpus = cpus;
> smp_cores = cores;
> smp_threads = threads;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature