Skip to contentSkip to conversation

v2: extend parser to parse pin: option

PATCH In review Andre Przywara 2 participants 4 messages Dec 16, 2008 – Dec 17, 2008

lore
Share & tools
lore

Andre Przywara

author
#0 lore raw

PATCH 4/8 v2: extend parser to parse pin: option

Signed-off-by Andre Przywara <[email protected]>
Signature · 9 lines
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
----to satisfy European Law for business letters:
AMD Saxony Limited Liability Company & Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

# HG changeset patch
# User Andre Przywara <[email protected]> # Date 1229432330 -3600
# Node ID 28e2cf1452bacb7c8fca7154cc259769bd9df79e # Parent c966a24fc826ccf77a698492003faeab87f0f9e5 add pin option to -numa cmdline parser

sysemu.h +4 -3
diff -r c966a24fc826 -r 28e2cf1452ba sysemu.h
@@ -97,9 +97,10 @@ extern int numnumanodes;
 extern int numnumanodes;
 extern uint64_t node_mem[MAX_NODES];
 extern uint64_t node_to_cpus[MAX_NODES];
-
-int parse_numa_args (const char *opt, uint64_t *mems,
-                     uint64_t *cpus, int maxentries, int expect_numnodes);
+extern uint64_t hostnodes[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;
vl.c +9 -4
diff -r c966a24fc826 -r 28e2cf1452ba vl.c
@@ -225,6 +225,7 @@ int numnumanodes = 0;
 int numnumanodes = 0;
 uint64_t node_mem[MAX_NODES];
 uint64_t node_to_cpus[MAX_NODES];
+uint64_t hostnodes[MAX_NODES];
 const char *vnc_display;
 int acpi_enabled = 1;
 int fd_bootchk = 1;

@@ -4520,7 +4521,7 @@ static int parse_to_array(const char *ar
     return num;
 }
 
-int parse_numa_args(const char *opt, uint64_t *mems,
+int parse_numa_args(const char *opt, uint64_t *hostnodes, uint64_t *mems,
                      uint64_t *cpus, int maxentries, int expect_numnodes)
 {
 const char *s;

@@ -4547,7 +4548,10 @@ int num = 0;
                 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);

@@ -4657,6 +4661,7 @@ int main(int argc, char **argv, char **e
     for(i = 0; i < MAX_NODES; i++) {
         node_to_cpus[i] = 0;
         node_mem[i] = 0;
+        hostnodes[i] = (uint64_t)-1;
     }
 
     usb_devices_index = 0;

@@ -5115,12 +5120,12 @@ int main(int argc, char **argv, char **e
                 break;
             case QEMU_OPTION_numa:
                 if (numnumanodes > 0)
-                    parse_numa_args(optarg, node_mem,
+                    parse_numa_args(optarg, hostnodes, node_mem,
                         node_to_cpus, MAX_NODES, 0);
                 else
-                    numnumanodes = parse_numa_args(optarg,
+                    numnumanodes = parse_numa_args(optarg, hostnodes,
                         node_mem, node_to_cpus, MAX_NODES, 1);
-                numnumanodes = parse_numa_args(optarg,
+                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");
author
#2 lore raw
Quoted 5 lines · Anthony Liguori
Quoted 2 lines · Andre Przywara
Signed-off-by Andre Przywara <[email protected]>

I think we suggested that this should be specified in a different option than -numa to separate the host/guest configuration bits.

As I said in the beginning of 0/8, I don't think that makes much sense.
Do you want this to be independent of the rest of NUMA (guest) code? Then this is a different story.
The purpose of this code is to pin _multiple_ guest nodes to _multiple_ different host nodes, something that is quite intricate to do otherwise.

In a single node case you can easily use numactl (or whatever your OS provides). If you want to use this NUMA code on a single node guest anyway, you can easily say: "-numa 1,pin:2" or "-numa 1 -numa pin:2".

Separating this pinning part out of the whole NUMA guest code will increase code complexity and IMHO is not justified, as OS provided methods are more portable for the single node case (currently this pinning is Linux only, which is backed by the original KVM target).

I suppose -pin is too ambiguous, also you pin it to nodes, so something like -nodepin or -numapin would suggest itself. I just insert a space to the latter.

Please tell me your opinion (and maybe a concrete suggestion on which syntax to use).

Regards,
Andre.

P.S. Do you agree to Avi that the colon should be substituted by an equal-sign?

Signature · 9 lines
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
----to satisfy European Law for business letters:
AMD Saxony Limited Liability Company & Co. KG,
Wilschdorfer Landstr. 101, 01109 Dresden, Germany
Register Court Dresden: HRA 4896, General Partner authorized
to represent: AMD Saxony LLC (Wilmington, Delaware, US)
General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy
#3 lore raw
Quoted 8 lines · Andre Przywara
Quoted 5 lines · Anthony Liguori
Quoted 2 lines · Andre Przywara
Signed-off-by Andre Przywara <[email protected]>

I think we suggested that this should be specified in a different option than -numa to separate the host/guest configuration bits.

As I said in the beginning of 0/8, I don't think that makes much sense.
Do you want this to be independent of the rest of NUMA (guest) code? Then this is a different story.

I'm not sure I see what you're asking.

Quoted 1 line

The purpose of this code is to pin _multiple_ guest nodes to _multiple_ different host nodes, something that is quite intricate to do otherwise.

Right now, you say:

-numa 2,mem:1G;2G,cpu:0-1;2-3,pin:2;3

My most preferred syntax would be:

-numa node,mem=0G-1G,cpu=0-1,nodeid=0 -numa pin,nodeid=0,hostid=2 \
-numa node,mem=1G-3G,cpu=2-3,nodeid=1 -numa pin,nodeid=1,hostid=3

It's more typing, but it maps really well to how -net tap works and how -drive works. It has a lot less special characters too. I'm not saying this is the only right solution, but this is probably what I would do if I were writing this.

Quoted 8 lines

Please tell me your opinion (and maybe a concrete suggestion on which syntax to use).

Regards,
Andre.

P.S. Do you agree to Avi that the colon should be substituted by an equal-sign?

Yes.

Regards,

Anthony Liguori