[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 1/3] NUMA: add -numa command line option
|
From: |
Anthony Liguori |
|
Subject: |
[Qemu-devel] Re: [PATCH 1/3] NUMA: add -numa command line option |
|
Date: |
Fri, 12 Dec 2008 08:56:59 -0600 |
|
User-agent: |
Thunderbird 2.0.0.17 (X11/20080925) |
Andre Przywara wrote:
This adds and parses a -numa command line option to QEMU.
Signed-off-by: Andre Przywara <address@hidden>
# HG changeset patch
# User Andre Przywara <address@hidden>
# Date 1228991781 -3600
# Node ID 394d02758aa4358be3bcd14f9d59efaf42e89328
# Parent b3a4224604d267a0e406a6d6809947f342a6b2ed
introduce -numa command line option
diff -r b3a4224604d2 -r 394d02758aa4 sysemu.h
--- a/sysemu.h Thu Dec 11 11:22:27 2008 +0100
+++ b/sysemu.h Thu Dec 11 11:36:21 2008 +0100
@@ -92,6 +92,16 @@ extern int alt_grab;
extern int alt_grab;
extern int usb_enabled;
extern int smp_cpus;
+
+#define MAX_NODES 64
+extern int numnumanodes;
+extern uint64_t hostnodes[MAX_NODES];
This should not be in this patch. This patch should just contain the
bits needed to create a virtual guest NUMA topology. The hostnode stuff
should be a separate patch.
+extern uint64_t node_mem[MAX_NODES];
+extern uint64_t node_to_cpus[MAX_NODES];
+
+int parse_numa_args (const char *opt, uint64_t *hostnodes, uint64_t *mems,
+ uint64_t *cpus, int maxentries, int expect_numnodes);
+
extern int cursor_hide;
extern int graphic_rotate;
extern int no_quit;
diff -r b3a4224604d2 -r 394d02758aa4 vl.c
--- a/vl.c Thu Dec 11 11:22:27 2008 +0100
+++ b/vl.c Thu Dec 11 11:36:21 2008 +0100
@@ -222,6 +222,10 @@ int win2k_install_hack = 0;
#endif
int usb_enabled = 0;
int smp_cpus = 1;
+int numnumanodes = 0;
+uint64_t hostnodes[MAX_NODES];
+uint64_t node_mem[MAX_NODES];
+uint64_t node_to_cpus[MAX_NODES];
const char *vnc_display;
int acpi_enabled = 1;
int fd_bootchk = 1;
@@ -3968,6 +3972,10 @@ static void help(int exitcode)
"-daemonize daemonize QEMU after initializing\n"
#endif
"-option-rom rom load a file, rom, into the option ROM space\n"
+ "-numa
nrnodes[,mem:size1[;size2..]][,cpu:cpu1[;cpu2..]][,pin:node1[;node2]]\n"
+ " create a multi NUMA node guest and optionally pin it
to\n"
+ " to the given host nodes. If mem and cpu are
omitted,\n"
+ " resources are split equally\n"
You're whitespace damaged.
+
+#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;
You're really whitespace damaged.
+ for (s = arg; s != NULL && *s != 0; s++) {
+ val = strtoull (s, &end, 10);
+ if (end == s && *s != '*') {num++; continue;}
+ if (num >= maxentries) break;
+ 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;
+ }
+ array[num++] = val;
+ if ((s = strchr (end, delim)) == NULL) break;
+ }
+ return num;
+}
This syntax is sufficiently complex that it needs thorough documentation
(in qemu-doc.texi).
+int parse_numa_args (const char *opt, uint64_t *hostnodes, uint64_t *mems,
+ uint64_t *cpus, int maxentries, int expect_numnodes)
+{
+const char *s;
+char *arg, *val, *end, *strtokarg;
+int num = 0;
+
+ arg = strdup(opt); strtokarg = arg;
+ if (expect_numnodes) {
+ s = strtok(strtokarg, ",");
strtok is generally evil. strsep() is a better choice.
+ if (s == NULL) {free (arg); return -1;}
Don't do this one on line.
+ num = strtol (s, &end, 10);
+ if (s == end) {free (arg); return -1;}
+ strtokarg = NULL;
+ }
+ while ((s=strtok(strtokarg, ","))!=NULL) {
+ strtokarg = 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);
+ } else if (!strcmp (s, "pin") && hostnodes != NULL) {
+ parse_to_array (val, hostnodes, ';', maxentries, 0);
+ }
+ }
+ }
+ free (arg);
+ return num;
+
Please try to make things make the rest of qemu better. You have a lot
of weird space after function names, etc. Consistency in large projects
is important.
int main(int argc, char **argv, char **envp)
{
@@ -4556,6 +4648,12 @@ 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++) {
+ hostnodes[i] = (uint64_t)-1;
+ node_to_cpus[i] = 0;
+ node_mem[i] = 0;
+ }
usb_devices_index = 0;
@@ -5011,6 +5109,14 @@ int main(int argc, char **argv, char **e
exit(1);
}
break;
+ case QEMU_OPTION_numa:
+ numnumanodes = parse_numa_args (optarg,
+ hostnodes, 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 +5257,24 @@ int main(int argc, char **argv, char **e
monitor_device = "stdio";
}
+ if (numnumanodes > 0) {
+ int i;
+
+ if (numnumanodes > smp_cpus)
+ numnumanodes = smp_cpus;
Why have this limitation? We would like to see CPU-less nodes supported
either as memory-only nodes or as IO nodes.
Which leads me to the question of how to plan on describing which nodes
have what hardware?
Regards,
Anthony Liguori