Re: [Qemu-devel] [RFC PATCH v0] numa: API to lookup NUMA node by address

[Paolo Bonzini <pbonzini@redhat.com>]

On 13/05/2015 20:06, Eduardo Habkost wrote:
> Also, this introduces a circular dependency between pc-dimm.c and
> numa.c. Instead of that, pc-dimm could simply notify us when a new
> device is realized (with just (addr, end, node) as arguments), so we can
> save the list of memory ranges inside struct node_info.
> 
> I wonder if the memory API already provides something that would help
> us. Paolo, do you see a way we could simply use a MemoryRegion as input
> to lookup the NUMA node?

No, but I guess you could add a numa_get/set_memory_region_node_id API
that uses a hash table.  That's a variant of the "pc-dimm could simply
notify" numa.c that you propose above.

Paolo

> 
>> +    qapi_free_MemoryDeviceInfoList(info_list);
>> +    error_setg(errp, "Address 0x" RAM_ADDR_FMT " doesn't belong to any "
>> +                "NUMA node", addr);
>> +
>> +    return -1;
>> +}
>> +
>> +static void numa_set_mem_address(int nodenr)
>> +{
>> +    if (nodenr) {
>> +        numa_info[nodenr].mem_start = numa_info[nodenr-1].mem_end;
> 
> You isolated the code inside a function, but it requires the function to
> be called in a specific nodenr order. I would just make it a loop that
> calculates mem_start and mem_end for all nodes, then you won't need a
> special case for node 0.
> 
>> +    } else {
>> +        numa_info[nodenr].mem_start = 0;
>> +    }
>> +    numa_info[nodenr].mem_end = numa_info[nodenr].mem_start +
>> +                                   numa_info[nodenr].node_mem;
> 
> Now that we have specific fields for the memory ranges, it would be
> interesting to reuse mem_start and mem_end inside
> memory_region_allocate_system_memory() instead of duplicating the
> address calculation there.
> 
>> +}
>> +
>>  static void numa_node_parse(NumaNodeOptions *node, QemuOpts *opts, Error **errp)
>>  {
>>      uint16_t nodenr;
>> @@ -276,6 +333,10 @@ void parse_numa_opts(MachineClass *mc)
>>          }
>>  
>>          for (i = 0; i < nb_numa_nodes; i++) {
>> +            numa_set_mem_address(i);
>> +        }
>> +
>> +        for (i = 0; i < nb_numa_nodes; i++) {
>>              if (!bitmap_empty(numa_info[i].node_cpu, MAX_CPUMASK_BITS)) {
>>                  break;
>>              }
>> -- 
>> 2.1.0
>>
>>
>