Re: [Qemu-devel] [PATCH 2/2 v5] numa: enable sparse node numbering on ppc

[Eduardo Habkost]

On Tue, Jul 01, 2014 at 01:13:28PM -0700, Nishanth Aravamudan wrote:
[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 12472c6..cdefafe 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1121,6 +1121,18 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>      guest_info->ram_size = below_4g_mem_size + above_4g_mem_size;
>      guest_info->apic_id_limit = pc_apic_id_limit(max_cpus);
>      guest_info->apic_xrupt_override = kvm_allows_irq0_override();
> +    /* No support for sparse NUMA node IDs yet: */
> +    for (i = max_numa_nodeid - 1; i >= 0; i--) {
> +        /* Report large node IDs first, to make mistakes easier to spot */
> +        if (!numa_info[i].present) {
> +            error_report("numa: Node ID missing: %d", i);
> +            exit(EXIT_FAILURE);
> +        }
> +    }
> +
> +    /* This must be always true if all nodes are present */
> +    assert(num_numa_nodes == max_numa_nodeid);
> +

I wonder if there's a better place where we could put this check.


>      guest_info->numa_nodes = num_numa_nodes;
>      guest_info->node_mem = g_malloc0(guest_info->numa_nodes *
>                                      sizeof *guest_info->node_mem);
[...]
> diff --git a/numa.c b/numa.c
> index 5930df0..a689e52 100644
> --- a/numa.c
> +++ b/numa.c
[...]
> @@ -225,9 +220,12 @@ void set_numa_nodes(void)
>           * must cope with this anyway, because there are BIOSes out there in
>           * real machines which also use this scheme.
>           */
> -        if (i == num_numa_nodes) {
> -            for (i = 0; i < max_cpus; i++) {
> -                set_bit(i, numa_info[i % num_numa_nodes].node_cpu);
> +        if (i == max_numa_nodeid) {
> +            for (i = 0, j = 0; i < max_cpus; i++) {

Doesn't j need to be initialized to -1, here?

Except for that, patch looks good to me. But I would be more comfortable
with it if we had automated tests to help ensure we are not breaking
compatibility of existing NUMA command-line conbinations with these
changes.

> +                do {
> +                    j = (j + 1) % (max_numa_nodeid);
> +                } while (!numa_info[j].present);
> +                set_bit(i, numa_info[j].node_cpu);
>              }
>          }
>      }
[...]
> 

-- 
Eduardo