[Qemu-devel] [PATCH 4/8] v2: extend parser to parse pin: option

[Qemu-devel] [PATCH 4/8] v2: extend parser to parse pin: option

[Andre Przywara]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Signed-off-by: Andre Przywara <andre.przywara@amd.com>

-- 
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

[-- Attachment #2: qemunuma_v2_cmdline_pin.patch --]
[-- Type: text/x-patch, Size: 3096 bytes --]

# HG changeset patch
# User Andre Przywara <andre.przywara@amd.com>
# Date 1229432330 -3600
# Node ID 28e2cf1452bacb7c8fca7154cc259769bd9df79e
# Parent  c966a24fc826ccf77a698492003faeab87f0f9e5
add pin option to -numa cmdline parser

diff -r c966a24fc826 -r 28e2cf1452ba sysemu.h
--- a/sysemu.h	Tue Dec 16 13:51:18 2008 +0100
+++ b/sysemu.h	Tue Dec 16 13:58:50 2008 +0100
@@ -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;
diff -r c966a24fc826 -r 28e2cf1452ba vl.c
--- a/vl.c	Tue Dec 16 13:51:18 2008 +0100
+++ b/vl.c	Tue Dec 16 13:58:50 2008 +0100
@@ -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");

[Qemu-devel] Re: [PATCH 4/8] v2: extend parser to parse pin: option

[Anthony Liguori]

Andre Przywara wrote:
> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
>

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

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 4/8] v2: extend parser to parse pin: option

[Andre Przywara]

Anthony Liguori wrote:
> Andre Przywara wrote:
>> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
>>
> 
> 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?

-- 
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

[Qemu-devel] Re: [PATCH 4/8] v2: extend parser to parse pin: option

[Anthony Liguori]

Andre Przywara wrote:
> Anthony Liguori wrote:
>> Andre Przywara wrote:
>>> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
>>>
>>
>> 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.

> 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.
>
> 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