[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 1/4] added -numa cmdline parameter parser
From: |
Andre Przywara |
Subject: |
[Qemu-devel] Re: [PATCH 1/4] added -numa cmdline parameter parser |
Date: |
Tue, 31 Mar 2009 22:34:40 +0200 |
User-agent: |
Thunderbird 2.0.0.18 (X11/20081105) |
Anthony Liguori wrote:
Andre Przywara wrote:
diff --git a/sysemu.h b/sysemu.h
index 3eab34b..b83a66c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -108,6 +108,11 @@ extern const char *bootp_filename;
+extern uint64_t node_cpumask[MAX_NODES];
This is going to cause some pain because it won't be long before someone
wants to support more than 64 cpus. I think there are two
possibilities. We could go the cpuset route and introduce a type with
special accessors to store a CPU bitmap.
Right, I was thinking about that one, too. I couldn't find an already
defined type for this, so I went the easy way for the first version of
the patch to make a review easier. Please note that the interface to the
BIOS is not limited in any way (beside a max of 2**64 nodes), so I could
sent a patch to overcome this limitation later (I suppose more than 64
vCPUs break something in other parts of code before that).
Or, we could rely on the property that each CPU can only be part of one
node and make the node association part of the CPUState. If for some
reason it's necessary to enumerate all of the CPUs for a given node, we
would have to walk the CPU list to get at that information. I don't
think that'll be a common thing though.
Sounds reasonable, I will take a look at it.
+static void numa_add(const char* optarg)
+{
+char option[128];
+char *endptr;
+unsigned long long value, endvalue;
+int nodenr;
That doesn't seem right indent-wise.
I knew I missed something....
+ /* assigning the VCPUs round-robin is easier to implement,
guest OSes
+ * must cope with this anyway, because there are BIOSes out
there in
+ * real machines which also use this scheme.
+ */
+ if (i == nb_numa_nodes) {
+ for (i = 0; i < smp_cpus; i++) {
+ node_cpumask[i % nb_numa_nodes] |= 1<<i;
+ }
+ }
The only thing that I don't like about this is that I don't think the
current -numa syntax can be used to describe a round-robin allocation.
IIUC, you can say -numa cpus=3 or -numa cpus=3-4 but there's no way to
say -numa cpus=3:5.
That means that if we ever change the default behavior, there's no way
that a management app could recreate the guest with that particular
topology (think live migration).
Good point, I was also not happy with the missing possibility to just
specify a list of vCPUs (since we already used the comma). If you think
that the colon could be valid delimiter here, I can introduce that (like
-numa cpus=0:4:8). That doesn't look very neat, so shall we use the
colon to separate the various numa sub-parameters (exchange comma and
colon)?
Thanks for the review and the comments!
Andre.
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Jochen Polster; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
[Qemu-devel] Re: [PATCH 1/4] added -numa cmdline parameter parser, Anthony Liguori, 2009/03/31
- [Qemu-devel] Re: [PATCH 1/4] added -numa cmdline parameter parser,
Andre Przywara <=
[Qemu-devel] Re: [PATCH 0/4] add NUMA emulation, Anthony Liguori, 2009/03/31