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
>>
>>
>
