qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 1/8] v2: introduce -numa command line option


From: Anthony Liguori
Subject: [Qemu-devel] Re: [PATCH 1/8] v2: introduce -numa command line option
Date: Tue, 16 Dec 2008 15:13:57 -0600
User-agent: Thunderbird 2.0.0.17 (X11/20080925)

Andre Przywara wrote:
Signed-off-by: Andre Przywara <address@hidden>

# HG changeset patch
# User Andre Przywara <address@hidden>
# 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<<i;

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]