Skip to contentSkip to conversation

Ping: c/s 20526 (tools: avoid cpu over-commitment if numa=on)

PATCH New Jan Beulich 3 participants 4 messages Jan 13, 2010

lore
Share & tools
lore

Jan Beulich

author
#0 lore raw

Andre,

I'm afraid this change isn't really correct:

Review

Quoted 2 lines
(quoted diff) +2 -0
+                cores_per_node = info['nr_cpus'] / info['nr_nodes']
+                nodes_required = (self.info['VCPUs_max'] + cores_per_node - 1) / cores_per_node

Simply using cores_per_node (as calculated here) as a divisor is bound to cause division-by-zero issues, namely when limiting the number of CPUs on the Xen command line (maxcpus=). I'm not sure though, what a reasonable solution to this might look like, since cores-per-node is a meaningless thing in an artificial setup like this, and may also be meaningless in asymmetric configurations. So perhaps we really need to iterate over nodes while summing up the number of CPUs they have until the number of needed vCPU-s was reached.

Jan

#1 lore raw
Quoted 8 lines · Jan Beulich

Andre,

I'm afraid this change isn't really correct:

Quoted 3 lines
(quoted diff) +2 -0
+                cores_per_node = info['nr_cpus'] / info['nr_nodes']
+                nodes_required = (self.info['VCPUs_max'] + cores_per_node -

1) / cores_per_node

Quoted 8 lines

Simply using cores_per_node (as calculated here) as a divisor is bound to cause division-by-zero issues, namely when limiting the number of CPUs on the Xen command line (maxcpus=). I'm not sure though, what a reasonable solution to this might look like, since cores-per-node is a meaningless thing in an artificial setup like this, and may also be meaningless in asymmetric configurations. So perhaps we really need to iterate over nodes while summing up the number of CPUs they have until the number of needed vCPU-s was reached.

Yes, please!

K.

#2 lore raw
In reply to Jan Beulich
Quoted 7 lines · Jan Beulich

Andre,

I'm afraid this change isn't really correct:

Review

Quoted 2 lines
(quoted diff) +2 -0
+                cores_per_node = info['nr_cpus'] / info['nr_nodes']
+                nodes_required = (self.info['VCPUs_max'] + cores_per_node - 1) / cores_per_node
Quoted 8 lines

Simply using cores_per_node (as calculated here) as a divisor is bound to cause division-by-zero issues, namely when limiting the number of CPUs on the Xen command line (maxcpus=). I'm not sure though, what a reasonable solution to this might look like, since cores-per-node is a meaningless thing in an artificial setup like this, and may also be meaningless in asymmetric configurations. So perhaps we really need to iterate over nodes while summing up the number of CPUs they have until the number of needed vCPU-s was reached.

Thanks for the heads-up. I have created a better patch basically doing what you described, actually it is even more elegant than the original version. I am about to test it and will send it out as soon as the testing is finished.

Regards,
Andre.

Signature · 9 lines
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#3 lore raw

PATCH 1/1 NUMA: Fix computation of needed nodes

In reply to Jan Beulich

Hi,

as Jan Beulich pointed out:

Quoted 8 lines

I'm afraid this change isn't really correct:

Review

Quoted 2 lines
(quoted diff) +2 -0
+                cores_per_node = info['nr_cpus'] / info['nr_nodes']
+                nodes_required = (self.info['VCPUs_max'] + cores_per_node - 1) / cores_per_node

Simply using cores_per_node (as calculated here) as a divisor is bound to cause division-by-zero issues, namely when limiting the number of CPUs on the Xen command line (maxcpus=).

Actually Jan's proposed method of getting additional nodes is more elegant, so I implemented: enumerating the best nodes and adding CPU affinity until all VCPUs can be backed by at least on physical core. This should fix problems with asymmetric NUMA configurations and cropped number of CPUs in Xen.

Regards,
Andre.

Signed-off-by Andre Przywara <[email protected]>
Signature · 9 lines
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: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
tools/python/xen/xend/XendDomainInfo.py +6 -7
diff -r 13d4e78ede97 tools/python/xen/xend/XendDomainInfo.py
@@ -2724,13 +2724,12 @@
                         candidate_node_list.append(i)
                 best_node = find_relaxed_node(candidate_node_list)[0]
                 cpumask = info['node_to_cpu'][best_node]
-                cores_per_node = info['nr_cpus'] / info['nr_nodes']
-                nodes_required = (self.info['VCPUs_max'] + cores_per_node - 1) / cores_per_node
-                if nodes_required > 1:
-                    log.debug("allocating %d NUMA nodes", nodes_required)
-                    best_nodes = find_relaxed_node(filter(lambda x: x != best_node, range(0,info['nr_nodes'])))
-                    for i in best_nodes[:nodes_required - 1]:
-                        cpumask = cpumask + info['node_to_cpu'][i]
+                best_nodes = find_relaxed_node(filter(lambda x: x != best_node, range(0,info['nr_nodes'])))
+                for node_idx in best_nodes:
+                    if len(cpumask) >= self.info['VCPUs_max']:
+                        break
+                    cpumask = cpumask + info['node_to_cpu'][node_idx]
+                    log.debug("allocating additional NUMA node %d", node_idx)
                 for v in range(0, self.info['VCPUs_max']):
                     xc.vcpu_setaffinity(self.domid, v, cpumask)
         return index

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