Konrad Rzeszutek Wilk wrote:
On Thu, Feb 04, 2010 at 10:54:22PM +0100, Andre Przywara wrote:
Konrad, thanks for your review. Comments inline.
This patch adds a "struct numa_guest_info" to libxc, which allows to
specify a guest's NUMA topology along with the appropriate host binding.
Here the information will be filled out by the Python wrapper (I left
out the libxl part for now), it will fill up the struct with sensible
default values (equal distribution), only the number of guest nodes
should be specified by the caller. The information is passed on to the
hvm_info_table. The respective memory allocation is in another patch.
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ...
+ if (numanodes > 1)
+ {
+ int vcpu_per_node, remainder;
+ int n;
+ vcpu_per_node = vcpus / numanodes;
+ remainder = vcpus % numanodes;
+ n = remainder * (vcpu_per_node + 1);
+ for (i = 0; i < vcpus; i++)
I don't know the coding style, but you seem to have two different
version of it. Here you do the 'for (i= ..')
It seems that Xen does not have a consistent coding style in this
respect, I have seen both versions in Xen already. I usually do
coding-style-by-copying, looking at similar nearby statements and
applying the style to the new one (useful if you do code changes in Xen
HV, tools, ioemu, etc.). That seemed to fail here.
Keir, is there a definite rule for the "space after brace" issue?
+ {
+ if (i < n)
+ numainfo.vcpu_to_node[i] = i / (vcpu_per_node + 1);
+ else
+ numainfo.vcpu_to_node[i] = remainder + (i - n) / vcpu_per_node;
+ }
+ for (i = 0; i < numanodes; i++)
+ numainfo.guest_to_host_node[i] = i % 2;
+ if ( nodemem_handle != NULL && PyList_Check(nodemem_handle) )
+ {
+ PyObject *item;
+ for ( i = 0; i < numanodes; i++)
and here it is ' for ( i = 0 ..')'.
I've failed in the past to find the coding style so I am not exactly
sure which one is correct.
+ {
+ item = PyList_GetItem(nodemem_handle, i);
+ numainfo.node_mem[i] = PyInt_AsLong(item);
+ fprintf(stderr, "NUMA: node %i has %lu kB\n", i,
+ numainfo.node_mem[i]);
There isn't any other way to print this out? Can't you use xc_dom_printf
routines?
Probably yes, although I am not sure whether this really belongs into
upstream code, I think this one is too verbose for most of the users, so
I consider this a leftover from debugging. I will remove it in the next
version.
+ }
+ } else {
+ unsigned int mempernode;
+ if ( nodemem_handle != NULL && PyInt_Check(nodemem_handle) )
+ mempernode = PyInt_AsLong(nodemem_handle);
+ else
+ mempernode = (memsize << 10) / numanodes;
+ fprintf(stderr, "NUMA: setting all nodes to %i KB\n", mempernode);
dittoo..
+ for (i = 0; i < numanodes; i++)
dittoo for the coding guideline.
+ numainfo.node_mem[i] = mempernode;
+ }
+ }
+
+ if ( xc_hvm_build_target_mem(self->xc_handle, dom, memsize, target,
+ &numainfo, image) != 0 )
and I guess here too
I am glad that most of your objections are coding-style related ;-)
return pyxc_error_to_exception();
#if !defined(__ia64__)
@@ -970,6 +1016,15 @@ static PyObject *pyxc_hvm_build(XcObject *self,
va_hvm->apic_mode = apic;
va_hvm->nr_vcpus = vcpus;
memcpy(va_hvm->vcpu_online, vcpu_avail, sizeof(vcpu_avail));
+
+ va_hvm->num_nodes = numanodes;
+ if (numanodes > 0) {
+ for (i = 0; i < vcpus; i++)
+ va_hvm->vcpu_to_node[i] = numainfo.vcpu_to_node[i];
+ for (i = 0; i < numanodes; i++)
+ va_hvm->node_mem[i] = numainfo.node_mem[i] >> 10;
Would it make sense to have 10 be #defined somewhere?
I think it is not worth, since it is just the conversion from MB to KB.
After all it maybe wiser to use a consistent unit in all parts of the
code. Will think about it.
+ }
+
for ( i = 0, sum = 0; i < va_hvm->length; i++ )
sum += ((uint8_t *)va_hvm)[i];
va_hvm->checksum -= sum;
@@ -1174,7 +1229,7 @@ static PyObject *pyxc_physinfo(XcObject *self)
nr_nodes++;
}
- ret_obj = Py_BuildValue("{s:i,s:i,s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s:s:s}",
+ ret_obj =
Py_BuildValue("{s:i,s:i,s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s,s:s}",
what changed in that line? My eyes don't seem to be able to find the
difference.
The last comma in the plus line was a colon in the minus line.
The Python doc says that these delimiters are ignored by the parser and
are just for the human eye. I was confused on the first glance and found
that the next to last colon is wrong. Actually it does not belong in
this patch, but I didn't found it worth to be a separate patch.
Regards,
Andre.
--
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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home