: xl: fix broken cpupool-numa-split
PATCH 1/1 : xl: fix broken cpupool-numa-split
Hi,
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one. On my machine the current output looks like:
root@dosorca:/data/images# xl cpupool-numa-split libxl: error: libxl.c:2803:libxl_create_cpupool Could not create cpupool error on creating cpupool root@dosorca:/data/images# xl cpupool-list Name CPUs Sched Active Domain count Pool-node0 42 credit y 1 Pool-node1 0 credit y 0 root@dosorca:/data/images# xl cpupool-list -c Name CPU list Pool-node0 0,1,2,3,4,5,6,7,8,9,10,11,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47 Pool-node1
The patch fixes this by using two poolid variables, now it looks like:
root@dosorca:/data/images# xl cpupool-numa-split root@dosorca:/data/images# xl cpupool-list Name CPUs Sched Active Domain count Pool-node0 6 credit y 1 Pool-node1 6 credit y 0 Pool-node2 6 credit y 0 Pool-node3 6 credit y 0 Pool-node4 6 credit y 0 Pool-node5 6 credit y 0 Pool-node6 6 credit y 0 Pool-node7 6 credit y 0 root@dosorca:/data/images# xl cpupool-list -c Name CPU list Pool-node0 0,1,2,3,4,5 Pool-node1 6,7,8,9,10,11 Pool-node2 12,13,14,15,16,17 Pool-node3 18,19,20,21,22,23 Pool-node4 24,25,26,27,28,29 Pool-node5 30,31,32,33,34,35 Pool-node6 36,37,38,39,40,41 Pool-node7 42,43,44,45,46,47
Please apply to Xen 4.1.0-rc
Regards,
Andre.
Signature · 2 lines
Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany
tools/libxl/xl_cmdimpl.c +9 -7
@@ -5716,7 +5716,7 @@ int main_cpupoolnumasplit(int argc, char **argv)int p;int c;int n; -uint32_t poolid; +uint32_t poolid_root, poolid_new;int schedid;int n_pools;int node;
@@ -5743,7 +5743,7 @@ int main_cpupoolnumasplit(int argc, char **argv)fprintf (stderr,"error getting cpupool info\n" );return -ERROR_NOMEM ; } - poolid = poolinfo[0 ].poolid; + poolid_root = poolinfo[0 ].poolid; schedid = poolinfo[0 ].sched_id;for (p =0 ; p < n_pools; p++) {libxl_cpupoolinfo_destroy (poolinfo + p);
@@ -5768,13 +5768,13 @@ int main_cpupoolnumasplit(int argc, char **argv)a cpupool without cpus in between */ node = topology.nodemap.array[0 ]; -if (libxl_cpupool_cpuadd_node (&ctx,0 , node, &n)) { +if (libxl_cpupool_cpuadd_node (&ctx, poolid_root, node, &n)) {fprintf (stderr,"error on adding cpu to Pool 0\n" );return -ERROR_FAIL ; }snprintf (name,15 ,"Pool-node%d" , node); - ret = -libxl_cpupool_rename (&ctx, name,0 ); + ret = -libxl_cpupool_rename (&ctx, name, poolid_root);if (ret) {fprintf (stderr,"error on renaming Pool 0\n" ); goto out;
@@ -5792,7 +5792,7 @@ int main_cpupoolnumasplit(int argc, char **argv) } node = topology.nodemap.array[c]; - ret = -libxl_cpupool_cpuremove_node (&ctx,0 , node, &n); + ret = -libxl_cpupool_cpuremove_node (&ctx, poolid_root, node, &n);if (ret) {fprintf (stderr,"error on removing cpu from Pool 0\n" ); goto out;
@@ -5800,13 +5800,15 @@ int main_cpupoolnumasplit(int argc, char **argv)snprintf (name,15 ,"Pool-node%d" , node);libxl_uuid_generate (&uuid); - ret = -libxl_create_cpupool (&ctx, name, schedid, cpumap, &uuid, &poolid); + poolid_new =0 ; + ret = -libxl_create_cpupool (&ctx, name, schedid, cpumap, &uuid, + &poolid_new);if (ret) {fprintf (stderr,"error on creating cpupool\n" ); goto out; } - ret = -libxl_cpupool_cpuadd_node (&ctx,0 , node, &n); + ret = -libxl_cpupool_cpuadd_node (&ctx, poolid_new, node, &n);if (ret) {fprintf (stderr,"error on adding cpus to cpupool\n" ); goto out;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 5 lines · Andre Przywara
Hi,
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one.
Uhh, silly copy and paste error! I think it happened when I introduced libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0. See attached patch.
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
Correct copy&paste error: add node to the new pool instead of pool 0
tools/libxl/xl_cmdimpl.c Fri Jan 28 07:21:53 2011 +0100 renamed from tools/libxl/xl_cmdimpl.c Tue Jan 25 14:06:52 2011 +0000 +1 -1
@@ -5806,7 +5806,7 @@ int main_cpupoolnumasplit(int argc, char goto out; } - ret = -libxl_cpupool_cpuadd_node(&ctx, 0, node, &n); + ret = -libxl_cpupool_cpuadd_node(&ctx, poolid, node, &n); if (ret) { fprintf(stderr, "error on adding cpus to cpupool\n"); goto out;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 4 lines · Juergen Gross
Quoted 3 lines · Andre Przywara
Hi,
Quoted 8 lines
Quoted 3 lines
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one.
Uhh, silly copy and paste error! I think it happened when I introduced
libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0. See attached patch.
Re patch itself: Acked-by: George Dunlap <[email protected]>
Re the cpupool interface:
Hang on; if the root poolid is always 0, why does xc_cpupool_create() interpret a poolid of 0 as "XEN_SYSCTL_CPUPOOL_PAR_ANY"? Doesn't that mean that if you're trying to create a new cpupool from cpus in the root pool, that you might get cpus from other pools?
If not, what's the point of the "CPUPOOL_PAR_ANY" parsing?
-George
Quoted 15 lines · George Dunlap
Quoted 4 lines · Juergen Gross
Quoted 3 lines · Andre Przywara
Hi,
Quoted 8 lines
Quoted 3 lines
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one.
Uhh, silly copy and paste error! I think it happened when I introduced
libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0. See attached patch.
Quoted 8 lines
Re patch itself: Acked-by: George Dunlap<[email protected]>
Re the cpupool interface:
Hang on; if the root poolid is always 0, why does
xc_cpupool_create()interpret a poolid of 0 as "XEN_SYSCTL_CPUPOOL_PAR_ANY"? Doesn't that mean that if you're trying to create a new cpupool from cpus in the root pool, that you might get cpus from other pools?
No. It just creates a cpupool. As cpupool 0 is always created by the hypervisor, it can't be created via libxc. So specifying 0 as poolid selects the next free id.
The cpus are allocated later.
Quoted 2 lines
If not, what's the point of the "
CPUPOOL_PAR_ANY" parsing?
Perhaps you just don't care which poolid you get?
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
George Dunlap writes ("Re: [Xen-devel] [PATCH]: xl: fix broken cpupool-numa-split"):
Quoted 7 lines
On Fri, Jan 28, 2011 at 6:26 AM, Juergen Gross
Quoted 4 lines
Uhh, silly copy and paste error! I think it happened when I introduced
libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0. See attached patch.Re patch itself: Acked-by: George Dunlap <[email protected]>
Thanks, all. Applied.
Ian.
PATCH 1/1 xl: fix broken cpupool-numa-split (part 2)
In reply to Juergen GrossQuoted 10 lines · Juergen Gross
Quoted 5 lines · Andre Przywara
Hi,
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one.
Uhh, silly copy and paste error! I think it happened when I introduced
libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0.
Why do you save this value then?
poolid = poolinfo[0].poolid;
Reading this made me think it can be an arbitrary value.
Quoted 1 line
See attached patch.
Easier, but that only solves one part of the problem (not populating the newly created pool). The second bug still persists, because poolid is not zeroed out again after the first creation. So the second iteration will try to reuse the just assigned value and abort with an error.
Ian, please apply the attached patch on top of Jürgens one.
----------
Before the creation and population of a new CPU pool we have to clear the poolid variable, which still contains the value from the previous iteration. This fixes the execution of xl cpupool-numa-split on machines with more than two nodes.
Signature · 2 lines
Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany
tools/libxl/xl_cmdimpl.c +1 -0
@@ -5800,6 +5800,7 @@ int main_cpupoolnumasplit(int argc, char **argv)snprintf (name,15 ,"Pool-node%d" , node);libxl_uuid_generate (&uuid); + poolid =0 ; ret = -libxl_create_cpupool (&ctx, name, schedid, cpumap, &uuid, &poolid);if (ret) {fprintf (stderr,"error on creating cpupool\n" );
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Re: [PATCH] xl: fix broken cpupool-numa-split (part 2)
Quoted 8 lines · Andre Przywara
Quoted 7 lines · Juergen Gross
Quoted 5 lines · Andre Przywara
Hi,
the implementation of xl cpupool-numa-split is broken. It basically deals with only one poolid, but there are two to consider: the one from the original root CPUpool, the other from the newly created one.
Quoted 6 lines
Quoted 3 lines
Uhh, silly copy and paste error! I think it happened when I introduced
libxl_cpupool_cpuadd_node()...
The correction is much easier. The root poolid is always 0.Why do you save this value then?
poolid = poolinfo[0].poolid;
Reading this made me think it can be an arbitrary value.
Sorry, must be a relict...
Quoted 9 lines
Quoted 2 lines
See attached patch.
Easier, but that only solves one part of the problem (not populating the newly created pool). The second bug still persists, because poolid is not zeroed out again after the first creation. So the second iteration will try to reuse the just assigned value and abort with an error.
Ian, please apply the attached patch on top of Jürgens one.
Quoted 8 lines
----------
Before the creation and population of a new CPU pool we have to clear the poolid variable, which still contains the value from the previous iteration.
This fixes the execution of xl cpupool-numa-split on machines with more than two nodes.Signed-off-by Andre Przywara <[email protected]>
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
AppliedRe: [PATCH] xl: fix broken cpupool-numa-split (part 2)
Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl: fix broken cpupool-numa-split (part 2)"):
Quoted 6 lines · Andre Przywara
Quoted 5 lines
Before the creation and population of a new CPU pool we have to clear the poolid variable, which still contains the value from the previous iteration.
This fixes the execution of xl cpupool-numa-split on machines with more than two nodes.
Applied, thanks.
Ian.