Re: [PATCH v2 3/4] eal: implement functions for get/set thread affinity

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Tue, Apr 12, 2022 at 03:43:39AM -0700, Tyler Retzlaff wrote:

> diff --git a/lib/eal/windows/eal_lcore.c b/lib/eal/windows/eal_lcore.c
> index 476c2d2..4f2224e 100644
> --- a/lib/eal/windows/eal_lcore.c
> +++ b/lib/eal/windows/eal_lcore.c

... snip ...

>  int
>  eal_create_cpu_map(void)
>  {
>  	SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *infos, *info;
>  	DWORD infos_size;
>  	bool full = false;
> +	int ret = 0;
>  
> +	infos = NULL;
>  	infos_size = 0;
>  	if (!GetLogicalProcessorInformationEx(
>  			RelationNumaNode, NULL, &infos_size)) {
> @@ -62,7 +166,8 @@ struct cpu_map {
>  			log_early("Cannot get NUMA node info size, error %lu\n",
>  				GetLastError());
>  			rte_errno = ENOMEM;
> -			return -1;
> +			ret = -1;
> +			goto exit;
>  		}
>  	}
>  

it can't be seen in the context of this diff but there is a leak of
infos on an error path. a v3 of the series will be submitted to address
the half-conversion to ret = -1 goto exit pattern.

> @@ -83,52 +188,24 @@ struct cpu_map {
>  
>  	info = infos;
>  	while ((uint8_t *)info - (uint8_t *)infos < infos_size) {
> -		unsigned int node_id = info->NumaNode.NodeNumber;
> -		GROUP_AFFINITY *cores = &info->NumaNode.GroupMask;
> -		struct lcore_map *lcore;
> -		unsigned int i, socket_id;
> -
> -		/* NUMA node may be reported multiple times if it includes
> -		 * cores from different processor groups, e. g. 80 cores
> -		 * of a physical processor comprise one NUMA node, but two
> -		 * processor groups, because group size is limited by 32/64.
> -		 */
> -		for (socket_id = 0; socket_id < cpu_map.socket_count;
> -		    socket_id++) {
> -			if (cpu_map.sockets[socket_id].node_id == node_id)
> -				break;
> -		}
> -
> -		if (socket_id == cpu_map.socket_count) {
> -			if (socket_id == RTE_DIM(cpu_map.sockets)) {
> -				full = true;
> -				goto exit;
> -			}
> -
> -			cpu_map.sockets[socket_id].node_id = node_id;
> -			cpu_map.socket_count++;
> -		}
> -
> -		for (i = 0; i < EAL_PROCESSOR_GROUP_SIZE; i++) {
> -			if ((cores->Mask & ((KAFFINITY)1 << i)) == 0)
> -				continue;
> -
> -			if (cpu_map.lcore_count == RTE_DIM(cpu_map.lcores)) {
> -				full = true;
> -				goto exit;
> -			}
> -
> -			lcore = &cpu_map.lcores[cpu_map.lcore_count];
> -			lcore->socket_id = socket_id;
> -			lcore->core_id =
> -				cores->Group * EAL_PROCESSOR_GROUP_SIZE + i;
> -			cpu_map.lcore_count++;
> +		if (eal_create_lcore_map(info)) {
> +			full = true;
> +			break;
>  		}
>  
>  		info = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *)(
>  			(uint8_t *)info + info->Size);
>  	}
>  
> +	if (eal_query_group_affinity()) {
> +		/*
> +		 * No need to set rte_errno here.
> +		 * It is set by eal_query_group_affinity().
> +		 */
> +		ret = -1;
> +		goto exit;
> +	}
> +
>  exit:
>  	if (full) {
>  		/* Not a fatal error, but important for troubleshooting. */