Re: [dpdk-dev] [PATCH 18.05 v4] eal: add function to return number of detected sockets

[gowrishankar muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>]

On Wednesday 21 March 2018 03:54 PM, Burakov, Anatoly wrote:
>
>>> +    config->numa_node_count = max_socket_id + 1;
>>
>> In some IBM servers, socket ID number does not seem to be in 
>> sequence. For an instance, 0 and 8 for a 2 node server.
>>
>> In this case, numa_node_count would mislead users if wrongly 
>> understood by its variable name IMO (see below)
>>> +    RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", 
>>> config->numa_node_count);
>>
>> For an instance, reading above message would tell 'EAL detected 8 
>> nodes' in my server, but actually there are only two nodes.
>>
>> Could its name better be 'numa_node_id_max' ?. Also, we store in 
>> actual count of numa nodes in _count variable.
>>
>> Also, there could be a case when there is no local memory available 
>> to a numa node too.
>>
>> Thanks,
>> Gowrishankar
>
> The point of this patchset is to (pre)allocate memory only on existing 
> sockets.
>
> If we don't know how many sockets there are, we are forced to 
> preallocate VA space per each *possible* NUMA node - that is, reserve 
> e.g. 8x128G of memory, 6 of which will go unused on a 2-socket system. 
> We can't know if there is no memory on socket in advance, but we can 
> at least avoid preallocating VA space for sockets that don't exist in 
> the first place.
>

Sounds good Anatoly.
May be, sysfs/ might help to confirm if a numa node has local memory ?.

Anyway, for the context of this particular patch (return numa nodes), 
below approach you mentioned is good.

> How about we store all possible socket id's instead? e.g. something like:
>
> static int numa_node_ids[MAX_NUMA_NODES];
> <...>
> int rte_eal_cpu_init() {
>     int sockets[RTE_MAX_LCORE];
>     <...>
>     for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
>         core_to_socket[lcore_id] = socket;
sockets[lcore_id] = eal_cpu_socket_id(lcore_id);
>     }
>     <...>
>     qsort(sockets);
>     <...>
>     // store all unique sockets in numa_node_ids in ascending order

Just thinking that, is there a purpose of retaining a numa ID which does 
not have local memory attached ?
but sockets[] is suppose to reflect all available nodes though (and 
assuming, its calling place to ensure
for the existence of numa local memory).


> }
> <...>
>
> on a 2 socket system we then get:
>
> rte_num_sockets() => return 2
> rte_get_socket_id(int idx) => return numa_node_ids[idx]
rte_get_socket_mem(idx) might help to validate for local memory existence ?

>
> Would that be suitable?
>

Thanks,
Gowrishankar