Re: [PATCH v4 2/3] eal: implement functions for get/set thread affinity

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2022-04-26 00:50 (UTC-0700), Tyler Retzlaff:
> Implement functions for getting/setting thread affinity.
> Threads can be pinned to specific cores by setting their
> affinity attribute.
> 
> Windows error codes are translated to errno-style error codes.
> The possible return values are chosen so that we have as
> much semantic compatibility between platforms as possible.
> 
> note: convert_cpuset_to_affinity has a limitation that all cpus of
> the set belong to the same processor group.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
with two minor issues below.

> +static int
> +eal_query_group_affinity(void)
> +{
> +	SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *infos = NULL;
> +	unsigned int *cpu_count = &cpu_map.cpu_count;
> +	DWORD infos_size = 0;
> +	int ret = 0;
> +	USHORT group_count;
> +	KAFFINITY affinity;
> +	USHORT group_no;
> +	unsigned int i;
> +
> +	if (!GetLogicalProcessorInformationEx(RelationGroup, NULL,
> +			&infos_size)) {
> +		DWORD error = GetLastError();
> +		if (error != ERROR_INSUFFICIENT_BUFFER) {
> +			log_early("Cannot get group information size, error %lu\n", error);

This code is now called not only at initialization,
but also after the logging is set up,
in which case this line will not go into a redirected destination.

I think there should not be log_early() at all in EAL,
rte_log() could work from the very beginning.
We're not in the kernel, stderr is available at once.

So I would work around with a static flag maybe.

[...]
> +static int
> +convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
> +		PGROUP_AFFINITY affinity)
> +{
> +	int ret = 0;
> +	PGROUP_AFFINITY cpu_affinity = NULL;
> +	unsigned int cpu_idx;
> +
> +	memset(affinity, 0, sizeof(GROUP_AFFINITY));
> +	affinity->Group = (USHORT)-1;
> +
> +	/* Check that all cpus of the set belong to the same processor group and
> +	 * accumulate thread affinity to be applied.
> +	 */
> +	for (cpu_idx = 0; cpu_idx < CPU_SETSIZE; cpu_idx++) {
> +		if (!CPU_ISSET(cpu_idx, cpuset))
> +			continue;
> +
> +		cpu_affinity = eal_get_cpu_affinity(cpu_idx);
> +
> +		if (affinity->Group == (USHORT)-1) {
> +			affinity->Group = cpu_affinity->Group;
> +		} else if (affinity->Group != cpu_affinity->Group) {
> +			ret = ENOTSUP;

Maybe worth a dedug log because the limitation is non-obvious.