On 01/22/2014 04:45 AM, Osier Yang wrote:
> There are 2 issues here: First we shouldn't add "1" to the return
> value of numa_max_node(), since the semanteme of the error message
s/semanteme/semantics/
> was changed, it's not saying about the number of total NUMA nodes
> anymore. Second, the value of "bit" is the position of the first
> bit which exceeds either numa_max_node() or NUMA_NUM_NODES, it can
> be any number in the range, so saying "bigger than $bit" is quite
> confused now. For example, assuming there is a NUMA machine which
> has 10 NUMA nodes, and one specifies the "nodeset" as "0,5,88",
> the error message will be like:
>
> Nodeset is out of range, host cannot support NUMA node bigger than 88
>
> It sounds like all NUMA node number less than 88 is fine, but
> actually the maximum NUMA node number the machine supports is 9.
>
> This patch fixes the issues by removing the addition with "1" and
> simplifies the error message as "NUMA node $bit is out of range".
> ---
> src/util/virnuma.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
> - maxnode = numa_max_node() + 1;
> -
> /* Convert nodemask to NUMA bitmask. */
> nodemask_zero(&mask);
> bit = -1;
> while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
> - if (bit > maxnode || bit > NUMA_NUM_NODES) {
> + if (bit > numa_max_node() || bit > NUMA_NUM_NODES) {
This calls numa_max_node() in a loop, where we used to call it just
once. Since this is third-party code, do we know how efficient it is?
It may be smarter to cache the results once than to call every iteration
of the loop, to avoid O(n*2) behavior on large hosts.
For that matter, can't we just set the max node to the smaller of
numa_max_node() and NUMA_NUM_NODES up front, and avoid the || in the loop?
ACK if you can fix that up.
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Nodeset is out of range, host cannot support "
> - "NUMA node bigger than %d"), bit);
> + _("NUMA node %d is out of range"), bit);
> return -1;
> }
> nodemask_set(&mask, bit);
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list