From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LChEy-0000cO-P0 for qemu-devel@nongnu.org; Tue, 16 Dec 2008 16:14:04 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LChEy-0000cC-0n for qemu-devel@nongnu.org; Tue, 16 Dec 2008 16:14:04 -0500 Received: from [199.232.76.173] (port=60539 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LChEx-0000c9-RL for qemu-devel@nongnu.org; Tue, 16 Dec 2008 16:14:03 -0500 Received: from yx-out-1718.google.com ([74.125.44.155]:21501) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LChEx-0003zm-J2 for qemu-devel@nongnu.org; Tue, 16 Dec 2008 16:14:03 -0500 Received: by yx-out-1718.google.com with SMTP id 3so1549935yxi.82 for ; Tue, 16 Dec 2008 13:14:02 -0800 (PST) Message-ID: <49481A15.6080402@codemonkey.ws> Date: Tue, 16 Dec 2008 15:13:57 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <4947B784.8020300@amd.com> In-Reply-To: <4947B784.8020300@amd.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 1/8] v2: introduce -numa command line option Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andre Przywara Cc: qemu-devel@nongnu.org, Avi Kivity Andre Przywara wrote: > Signed-off-by: Andre Przywara > > # HG changeset patch > # User Andre Przywara > # Date 1229425990 -3600 > # Node ID 6690ab0a34fdedaebbfe1068dfe7351734e8a1d7 > # Parent 4271a7547668e4b02cae88e66c936d9cacab624e > add -numa command line option > > diff -r 4271a7547668 -r 6690ab0a34fd sysemu.h > > @@ -4065,6 +4072,7 @@ enum { > QEMU_OPTION_usb, > QEMU_OPTION_usbdevice, > QEMU_OPTION_smp, > + QEMU_OPTION_numa, > QEMU_OPTION_vnc, > QEMU_OPTION_no_acpi, > QEMU_OPTION_curses, > @@ -4171,6 +4179,7 @@ static const QEMUOption qemu_options[] = > { "win2k-hack", 0, QEMU_OPTION_win2k_hack }, > { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice }, > { "smp", HAS_ARG, QEMU_OPTION_smp }, > + { "numa", HAS_ARG, QEMU_OPTION_numa}, > { "vnc", HAS_ARG, QEMU_OPTION_vnc }, > #ifdef CONFIG_CURSES > { "curses", 0, QEMU_OPTION_curses }, > @@ -4456,6 +4465,94 @@ static void termsig_setup(void) > } > > #endif > + > +#define PARSE_FLAG_BITMASK 1 > +#define PARSE_FLAG_SUFFIX 2 > + > +static int parse_to_array(const char *arg, uint64_t *array, > + char delim, int maxentries, int flags) > +{ > + const char *s; > + char *end; > + int num = 0; > + unsigned long long int val,endval; > + > + for (s = arg; s != NULL && *s != 0; s++) { > + val = strtoull(s, &end, 10); > + if (end == s && *s != '*') { > + num++; continue; > + } > + if (num >= maxentries) break; > + if (*end == delim && (flags & PARSE_FLAG_SUFFIX)) > + val *= 1024 * 1024; > + switch (*end) { > + case 'g': > + case 'G': > + if (flags & PARSE_FLAG_SUFFIX) val *= 1024; > + /* fall through */ > + case 'm': > + case 'M': > + if (flags & PARSE_FLAG_SUFFIX) val *= 1024; > + /* fall through */ > + case 'k': > + case 'K': > + if (flags & PARSE_FLAG_SUFFIX) val *= 1024; > + break; > + case '*': > + val = (unsigned long long int)-1; > + break; > + case '-': > + if (!(flags & PARSE_FLAG_BITMASK)) break; > + s = end + 1; > + endval = strtoull(s, &end, 10); > + val = (1 << (endval + 1)) - (1 << val); > + break; > + case 0: > + if (flags & PARSE_FLAG_SUFFIX) val *= 1024 * 1024; > + /* fall through */ > + default: > + if (flags & PARSE_FLAG_BITMASK) val = 1 << val; > + break; > The fall throughs here are very confusion. No suffix means G or bitmask depending on the context? The indenting is really messed up in this function too. > +int parse_numa_args(const char *opt, uint64_t *mems, > + uint64_t *cpus, int maxentries, int expect_numnodes) > +{ > +const char *s; > +char *arg, *val, *end, *token; > +int num = 0; > This indenting is messed up. > + arg = strdup(opt); token = arg; > + if (expect_numnodes) { > + s = strsep(&token, ","); > + if (s == NULL) { > + free(arg); > + return -1; > + } > + num = strtol(s, &end, 10); > + if (s == end) { > + free(arg); > + return -1; > + } > + } > + while ((s=strsep(&token, ","))!=NULL) { > + if ((val = strchr(s, ':'))) { > + *val++ = 0; > + if (!strcmp(s, "mem") && mems != NULL) { > + parse_to_array(val, mems, ';', maxentries, PARSE_FLAG_SUFFIX); > + } else if (!strcmp(s, "cpu") && cpus != NULL) { > + parse_to_array(val, cpus, ';', maxentries, PARSE_FLAG_BITMASK); > + } > + } > + } > + free(arg); > + return num; > +} > > int main(int argc, char **argv, char **envp) > { > @@ -4556,6 +4653,11 @@ int main(int argc, char **argv, char **e > for(i = 1; i < MAX_PARALLEL_PORTS; i++) > parallel_devices[i] = NULL; > parallel_device_index = 0; > + > + for(i = 0; i < MAX_NODES; i++) { > + node_to_cpus[i] = 0; > + node_mem[i] = 0; > + } > > usb_devices_index = 0; > > @@ -5011,6 +5113,20 @@ int main(int argc, char **argv, char **e > exit(1); > } > break; > + case QEMU_OPTION_numa: > + if (numnumanodes > 0) > + parse_numa_args(optarg, node_mem, > + node_to_cpus, MAX_NODES, 0); > + else > + numnumanodes = parse_numa_args(optarg, > + node_mem, node_to_cpus, MAX_NODES, 1); > + numnumanodes = parse_numa_args(optarg, > + node_mem, node_to_cpus, MAX_NODES, 1); > + if (numnumanodes < 0) { > + fprintf(stderr, "Invalid number of NUMA nodes\n"); > + exit(1); > + } > + break; > case QEMU_OPTION_vnc: > vnc_display = optarg; > break; > @@ -5151,6 +5267,24 @@ int main(int argc, char **argv, char **e > monitor_device = "stdio"; > } > > + if (numnumanodes > 0) { > + int i; > + > + if (numnumanodes > smp_cpus) > + numnumanodes = smp_cpus; > + > + for (i = 0; i < numnumanodes; i++) if (node_mem[i] != 0) break; > Please split to multiple lines. > + if (i == numnumanodes) { > + for (i = 0; i < numnumanodes; i++) > + node_mem[i] = (ram_size / numnumanodes) & ~((1 << 20UL) - 1); > + } > + for (i = 0; i < numnumanodes; i++) if (node_to_cpus[i] != 0) break; > + if (i == numnumanodes) { > + for (i = 0; i < smp_cpus; i++) > + node_to_cpus[i % numnumanodes] |= 1< The way CPUs are allocate here seems strange? Each CPU is assigned round robin? Should you have node 0 contain 1..X, node 1 contain X..Y, node 2 contain Y..smp_cpus? Regards, Anthony Liguori