xl cpupool-numa-split: reduce number of Dom0 vcpus

PATCH In review Juergen Gross 3 participants 8 messages Feb 14, 2011 – Feb 17, 2011

Share & tools

Juergen Gross

author

2011-02-14 10:47 UTC #0 lore raw

PATCH 1/1 xl cpupool-numa-split: reduce number of Dom0 vcpus

When reducing the number of physical cpus available for Domain-0 by xl cpupool-numa-split, reduce the number of vcpus accordingly.

Signed-off-by [email protected]

1 file changed, 20 insertions(+), 2 deletions(-)

tools/libxl/xl_cmdimpl.c | 22 ++++++++++++++++++++--

# HG changeset patch
# User Juergen Gross <[email protected]> # Date 1297680409 -3600
# Node ID fdcb4e8eb4bb02f2c14b5333b9839dbd7b8786df # Parent f3579aa973c24a4ecee809a184b08c1855cd07ba xl cpupool-numa-split: reduce number of Dom0 vcpus

When reducing the number of physical cpus available for Domain-0 by xl cpupool-numa-split, reduce the number of vcpus accordingly.

Signed-off-by [email protected]

tools/libxl/xl_cmdimpl.c +20 -2

diff -r f3579aa973c2 -r fdcb4e8eb4bb tools/libxl/xl_cmdimpl.c
@@ -5762,6 +5762,7 @@ int main_cpupoolnumasplit(int argc, char
     libxl_cpumap cpumap;
     libxl_cpupoolinfo *poolinfo;
     libxl_topologyinfo topology;
+    libxl_dominfo info;
 
     while ((opt = getopt(argc, argv, "h")) != -1) {
         switch (opt) {

@@ -5817,11 +5818,28 @@ int main_cpupoolnumasplit(int argc, char
         goto out;
     }
 
+    n = 0;
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == node) {
             topology.nodemap.array[c] = LIBXL_CPUARRAY_INVALID_ENTRY;
-        }
-    }
+            libxl_cpumap_set(&cpumap, n);
+            n++;
+        }
+    }
+    if (libxl_set_vcpuonline(&ctx, 0, &cpumap)) {
+        fprintf(stderr, "error on removing vcpus for Domain-0\n");
+        goto out;
+    }
+    for (c = 0; c < 10; c++) {
+        if (libxl_domain_info(&ctx, &info, 0)) {
+            fprintf(stderr, "error on getting info for Domain-0\n");
+            goto out;
+        }
+        if (info.vcpu_online == n) {
+            break;
+        }
+    }
+    memset(cpumap.map, 0, cpumap.size);
 
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == LIBXL_CPUARRAY_INVALID_ENTRY) {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

George Dunlap

2011-02-14 12:36 UTC #1 lore raw

Is looping 10 times really the best option?

-George

Quoted 17 lines · Juergen Gross

When reducing the number of physical cpus available for Domain-0 by xl cpupool-numa-split, reduce the number of vcpus accordingly.
Signed-off-by [email protected]
1 file changed, 20 insertions(+), 2 deletions(-)
tools/libxl/xl_cmdimpl.c | 22 ++++++++++++++++++++--
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Juergen Gross

author

2011-02-14 13:13 UTC #2 lore raw

Quoted 1 line · George Dunlap

Is looping 10 times really the best option?

Oops! Forgot the sleep(1) :-(

What about this one?

Signature · 5 lines

Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: [email protected]
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

When reducing the number of physical cpus available for Domain-0 by xl cpupool-numa-split, reduce the number of vcpus accordingly.

Signed-off-by [email protected]

tools/libxl/xl_cmdimpl.c +20 -2

diff -r f3579aa973c2 -r fdcb4e8eb4bb tools/libxl/xl_cmdimpl.c
@@ -5762,6 +5762,7 @@ int main_cpupoolnumasplit(int argc, char
     libxl_cpumap cpumap;
     libxl_cpupoolinfo *poolinfo;
     libxl_topologyinfo topology;
+    libxl_dominfo info;
 
     while ((opt = getopt(argc, argv, "h")) != -1) {
         switch (opt) {

@@ -5817,11 +5818,28 @@ int main_cpupoolnumasplit(int argc, char
         goto out;
     }
 
+    n = 0;
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == node) {
             topology.nodemap.array[c] = LIBXL_CPUARRAY_INVALID_ENTRY;
-        }
-    }
+            libxl_cpumap_set(&cpumap, n);
+            n++;
+        }
+    }
+    if (libxl_set_vcpuonline(&ctx, 0, &cpumap)) {
+        fprintf(stderr, "error on removing vcpus for Domain-0\n");
+        goto out;
+    }
+    for (c = 0; c < 10; c++) {
+        if (libxl_domain_info(&ctx, &info, 0)) {
+            fprintf(stderr, "error on getting info for Domain-0\n");
+            goto out;
+        }
+        if (info.vcpu_online == n) {
+            break;
+        }
+    }
+    memset(cpumap.map, 0, cpumap.size);
 
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == LIBXL_CPUARRAY_INVALID_ENTRY) {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Ian Jackson

2011-02-14 15:45 UTC #3 lore raw

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-numa-split: reduce number of Dom0 vcpus"):

Quoted 5 lines · George Dunlap

Quoted 1 line
Is looping 10 times really the best option?
Oops! Forgot the sleep(1) :-(
What about this one?

Um, are you sure ? It still doesn't seem to have a sleep and you haven't answered George's question ...

Ian.

Juergen Gross

author

2011-02-15 06:02 UTC #4 lore raw

Quoted 9 lines · Ian Jackson

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-numa-split: reduce number of Dom0 vcpus"):
Quoted 5 lines · George Dunlap
Quoted 1 line
Is looping 10 times really the best option?
Oops! Forgot the sleep(1) :-(
What about this one?
Um, are you sure ? It still doesn't seem to have a sleep and you haven't answered George's question ...

Sigh. Modifying the patch isn't enough, have to send the modified version...

Regarding the question: libxl_set_vcpuonline() is asynchronous. It will trigger the offlining (or onlining) of vcpus, but the result is not checked and will happen sometimes in the future, if ever. So some kind of timeout seems to be a good idea. 10 seconds should be enough to do the work.

I added a failure exit if not all required vcpus are offlined.

Juergen

Signature · 5 lines

Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: [email protected]
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

When reducing the number of physical cpus available for Domain-0 by xl cpupool-numa-split, reduce the number of vcpus accordingly.

Signed-off-by [email protected]

tools/libxl/xl_cmdimpl.c +25 -2

diff -r f3579aa973c2 -r fdcb4e8eb4bb tools/libxl/xl_cmdimpl.c
@@ -5762,6 +5762,7 @@ int main_cpupoolnumasplit(int argc, char
     libxl_cpumap cpumap;
     libxl_cpupoolinfo *poolinfo;
     libxl_topologyinfo topology;
+    libxl_dominfo info;
 
     while ((opt = getopt(argc, argv, "h")) != -1) {
         switch (opt) {

@@ -5817,11 +5818,33 @@ int main_cpupoolnumasplit(int argc, char
         goto out;
     }
 
+    n = 0;
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == node) {
             topology.nodemap.array[c] = LIBXL_CPUARRAY_INVALID_ENTRY;
-        }
-    }
+            libxl_cpumap_set(&cpumap, n);
+            n++;
+        }
+    }
+    if (libxl_set_vcpuonline(&ctx, 0, &cpumap)) {
+        fprintf(stderr, "error on removing vcpus for Domain-0\n");
+        goto out;
+    }
+    for (c = 0; c < 10; c++) {
+        if (libxl_domain_info(&ctx, &info, 0)) {
+            fprintf(stderr, "error on getting info for Domain-0\n");
+            goto out;
+        }
+        if (info.vcpu_online == n) {
+            break;
+        }
+        sleep(1);
+    }
+    if (info.vcpu_online > n) {
+        fprintf(stderr, "failed to offline vcpus\n");
+        goto out;
+    }
+    memset(cpumap.map, 0, cpumap.size);
 
     for (c = 0; c < topology.nodemap.entries; c++) {
         if (topology.nodemap.array[c] == LIBXL_CPUARRAY_INVALID_ENTRY) {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Ian Jackson

2011-02-15 19:27 UTC #5 lore raw

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-numa-split: reduce number of Dom0 vcpus"):

Quoted 1 line

I added a failure exit if not all required vcpus are offlined.

Thanks. But, is this test correct ?

tools/libxl/xl_cmdimpl.c int main_cpupoolnumasplit(int argc, char · line 5840

Quoted 3 lines

tools/libxl/xl_cmdimpl.c +3 -0

+        if (info.vcpu_online == n) {
+            break;
+        }

Is it possible for the online map to have n cpus in it, but for them to be the wrong n cpus ?

What if two copies of this code run concurrently ? (Perhaps in 4.2 we will need to add a lock?)

Ian.

Juergen Gross

author

2011-02-16 08:40 UTC #6 lore raw

Quoted 11 lines · Ian Jackson

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-numa-split: reduce number of Dom0 vcpus"):
Quoted 1 line
I added a failure exit if not all required vcpus are offlined.
Thanks. But, is this test correct ?
tools/libxl/xl_cmdimpl.c int main_cpupoolnumasplit(int argc, char · line 5840
Quoted 3 lines
tools/libxl/xl_cmdimpl.c +3 -0
+        if (info.vcpu_online == n) {
+            break;
+        }
Is it possible for the online map to have n cpus in it, but for them to be the wrong n cpus ?

The number of online vcpus is important, not which vcpus.
In a strange case it could lead to a wrong number of online vcpus, if the number was correct in the beginning and e.g. vcpu 0 was offline and offlining the last vcpu fails. But I don't think this would be a huge problem, as offlining the vcpus is just a optimization measure.

Quoted 3 lines

What if two copies of this code run concurrently ? (Perhaps in 4.2 we will need to add a lock?)

The result of both copies should be the same, as it is depending on the machine configuration only.
And we are speaking of a global machine configuration action. This would mean two administrators are configuring the same machine without knowing what the other is doing. Seems to be a very strange scenario. A possible failure (meaning: not reaching the desired configuration, but no crashing) should be regarded as acceptable in this case.

Juergen

Signature · 5 lines

Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: [email protected]
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Ian Jackson

2011-02-17 19:39 UTC #7 lore raw

In reply to Juergen Gross

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-numa-split: reduce number of Dom0 vcpus"):

Quoted 6 lines

Regarding the question: libxl_set_vcpuonline() is asynchronous. It will trigger the offlining (or onlining) of vcpus, but the result is not checked and will happen sometimes in the future, if ever. So some kind of timeout seems to be a good idea. 10 seconds should be enough to do the work.
I added a failure exit if not all required vcpus are offlined.

Thanks, and for your replies to my questions. I have applied your patch.

Ian.