From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmPd8-0005yi-5e for qemu-devel@nongnu.org; Thu, 06 Nov 2014 11:09:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XmPd0-0002aC-Nu for qemu-devel@nongnu.org; Thu, 06 Nov 2014 11:09:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46967) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmPd0-0002a2-Gv for qemu-devel@nongnu.org; Thu, 06 Nov 2014 11:09:42 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA6G9fji011095 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 6 Nov 2014 11:09:41 -0500 From: Andrew Jones Date: Thu, 6 Nov 2014 17:09:35 +0100 Message-Id: <1415290175-17314-1-git-send-email-drjones@redhat.com> Subject: [Qemu-devel] [PATCH] vl: rework smp_parse List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: ehabkost@redhat.com 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 --- 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