2022-04-01 06:29 (UTC-0700), Tyler Retzlaff:
> Implement functions for getting/setting thread affinity.
> Threads can be pinned to specific cores by setting their
> affinity attribute.
>
> Signed-off-by: Narcisa Vasile <navas...@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com>
Acked-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>
Please see some small comments below.
> ---
> lib/eal/include/rte_thread.h | 45 ++++++++++
> lib/eal/unix/rte_thread.c | 16 ++++
> lib/eal/version.map | 4 +
> lib/eal/windows/eal_lcore.c | 173
> +++++++++++++++++++++++++++++----------
> lib/eal/windows/eal_windows.h | 10 +++
> lib/eal/windows/include/rte_os.h | 2 +
> lib/eal/windows/rte_thread.c | 131 ++++++++++++++++++++++++++++-
> 7 files changed, 336 insertions(+), 45 deletions(-)
>
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index 8be8ed8..4eb113f 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -2,6 +2,8 @@
> * Copyright(c) 2021 Mellanox Technologies, Ltd
> */
>
> +#include <stdint.h>
> +
> #include <rte_os.h>
> #include <rte_compat.h>
>
> @@ -21,6 +23,13 @@
> #endif
>
> /**
> + * Thread id descriptor.
> + */
> +typedef struct rte_thread_tag {
> + uintptr_t opaque_id; /**< thread identifier */
> +} rte_thread_t;
> +
> +/**
> * TLS key type, an opaque pointer.
> */
> typedef struct eal_tls_key *rte_thread_key;
> @@ -28,6 +37,42 @@
> #ifdef RTE_HAS_CPUSET
>
> /**
Missing a common part for experimental functions:
* @warning
* @b EXPERIMENTAL: this API may change without prior notice.
> + * Set the affinity of thread 'thread_id' to the cpu set
> + * specified by 'cpuset'.
> + *
> + * @param thread_id
> + * Id of the thread for which to set the affinity.
> + *
> + * @param cpuset
> + * Pointer to CPU affinity to set.
> + *
> + * @return
> + * On success, return 0.
> + * On failure, return a positive errno-style error number.
> + */
> +__rte_experimental
> +int rte_thread_set_affinity_by_id(rte_thread_t thread_id,
> + const rte_cpuset_t *cpuset);
> +
> +/**
Same here.
> + * Get the affinity of thread 'thread_id' and store it
> + * in 'cpuset'.
> + *
> + * @param thread_id
> + * Id of the thread for which to get the affinity.
> + *
> + * @param cpuset
> + * Pointer for storing the affinity value.
> + *
> + * @return
> + * On success, return 0.
> + * On failure, return a positive errno-style error number.
> + */
> +__rte_experimental
> +int rte_thread_get_affinity_by_id(rte_thread_t thread_id,
> + rte_cpuset_t *cpuset);
> +
> +/**
> * Set core affinity of the current thread.
> * Support both EAL and non-EAL thread and update TLS.
> *
[...]
> +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);
Please don't break string constants for easy search.
> + rte_errno = EINVAL;
> + ret = -1;
> + goto cleanup;
> + }
> + }
[...]
> +static bool
> +eal_create_lcore_map(const SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *info)
> +{
> + const unsigned int node_id = info->NumaNode.NodeNumber;
> + const GROUP_AFFINITY *cores = &info->NumaNode.GroupMask;
> + struct lcore_map *lcore;
> + unsigned int socket_id;
> + unsigned int i;
> +
> + /* 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;
> + }
Nit: multi-line comments should start with a line containing just "/*",
and {} are no needed here.
[...]
> +static int
> +rte_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 = EINVAL;
Should it be ENOTSUP, because we don't support this case?
This limitation is also worth documenting in the commit log.
> + goto cleanup;
> + }
> +
> + affinity->Mask |= cpu_affinity->Mask;
> + }
> +
> + if (affinity->Mask == 0) {
> + ret = EINVAL;
> + goto cleanup;
> + }
> +
> +cleanup:
> + return ret;
> +}
[...]
