From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1AD60A0546; Sun, 2 May 2021 02:42:04 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8C6F640140; Sun, 2 May 2021 02:42:03 +0200 (CEST) Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) by mails.dpdk.org (Postfix) with ESMTP id 3A6E04013F for ; Sun, 2 May 2021 02:42:02 +0200 (CEST) Received: by mail-lf1-f54.google.com with SMTP id j4so2829973lfp.0 for ; Sat, 01 May 2021 17:42:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=yWSKS3126WDQZW70aHncfH2kn3CXDOfobItD7drLi0U=; b=ocWinUkHrqpIsNf1PHSe2jjY5HvF7X0H/LkdiV2V0elTGwmhy9x2jNH7ulRdjGaaaf VWmeLd0VowPzDEJ/0BmUJXrD26Mej7WckXen8LMySof3Ftt1RIqMSwjkCLGDsjNDMTuw F7tlFDD6hyAfh7RjhoulgoHIygpJYa4GZVpo6m0FNvJdbjrzPHEoFebWhWXTHiXr/0b0 sdLYIzXp7OjKASOUkj6mZlYQpuNQQqbQ6ZbWaH/jXyAMoTQxPf+sZb6vmQr9f6uM1ITi JNj/XNoyMbwLkKNHys7ShI/9w1wRuXizr0KG1uTKyGCpJwWziZO4aAltz2fAyf0x+sOo DKww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=yWSKS3126WDQZW70aHncfH2kn3CXDOfobItD7drLi0U=; b=WyTlTZYiQWCOq9lb6BgWMJwb9Y8wni5QKrIGViQU16AyeSKmoUYMWN7yCxSfDD/Rcu 6pDopfCF+4Xqp6ZyLbKeQLga68aTHLQWaHD1IPDjIr6BJZqVv/nDt+6oOUqhVJaCWDuW KorkDyqUbQKDl56oIWAvnCmjGjMXGmRAnJfI+bhsIRkcLQZxzyVeK7Op3iOJJOxMSThp y4iGo3Gm/sB7geecL/LB+YDj/5ce1fvypHdn0VXMKyNQewDZp5Z0N0GhWMLz1CEup3b5 PomhZ6NISsbhvf98ZZHYPlYbDpr1VgtazoBSzUcBYDZeqQa2I5vRO9cQySjOdulx0N0/ Y8/g== X-Gm-Message-State: AOAM533gSppycluH5dLQQoubEwN+P19TTxnEBRMWD4f5RZ4m/3eAGNni TSmIB5J6EgFCwk/o2Sp7Cm0= X-Google-Smtp-Source: ABdhPJz4ywJbW8ka8GKaGFRk1FMGzt/SiknTz4a72QselM22Huze672yEOj4pYRubrSwj+34ZwfOjg== X-Received: by 2002:a05:6512:3981:: with SMTP id j1mr8128128lfu.530.1619916121720; Sat, 01 May 2021 17:42:01 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id b28sm626238lfo.213.2021.05.01.17.41.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 01 May 2021 17:42:00 -0700 (PDT) Date: Sun, 2 May 2021 03:41:59 +0300 From: Dmitry Kozlyuk To: Narcisa Ana Maria Vasile Cc: dev@dpdk.org, thomas@monjalon.net, khot@microsoft.com, navasile@microsoft.com, dmitrym@microsoft.com, roretzla@microsoft.com, talshn@nvidia.com, ocardona@microsoft.com, bruce.richardson@intel.com, david.marchand@redhat.com, pallavi.kadam@intel.com Message-ID: <20210502034159.49a8b323@sovereign> In-Reply-To: <1617413948-10504-5-git-send-email-navasile@linux.microsoft.com> References: <1617057640-24301-2-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-1-git-send-email-navasile@linux.microsoft.com> <1617413948-10504-5-git-send-email-navasile@linux.microsoft.com> X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 04/10] eal: implement functions for thread affinity management X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2021-04-02 18:39 (UTC-0700), Narcisa Ana Maria Vasile: [...] > +/** > + * 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_size > + * Size of the cpu set. > + * > + * @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, size_t cpuset_size, > + rte_cpuset_t *cpuset); Existing DPDK functions with `rte_cpuset_t` don't take `cpuset_size`. Is it really necessary here? > /** > * Initialize the attributes of a thread. > * These attributes can be passed to the rte_thread_create() function > diff --git a/lib/librte_eal/windows/eal_lcore.c b/lib/librte_eal/windows/eal_lcore.c > index a85149be9..023c5c895 100644 > --- a/lib/librte_eal/windows/eal_lcore.c > +++ b/lib/librte_eal/windows/eal_lcore.c [...] > + > +static bool > +eal_check_for_duplicate_numa(const SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *info) This function's purpose is not to check for "duplicate NUMA" (what's this?). Please find an eloquent name. The comment below just explained a case when a NUMA node may be reported as two logical processors due to processor group size limitiation. > +{ > + const unsigned int node_id = info->NumaNode.NodeNumber; > + const GROUP_AFFINITY *cores = &info->NumaNode.GroupMask; > + struct lcore_map *lcore; > + unsigned int 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)) > + return true; > + > + cpu_map.sockets[socket_id].node_id = node_id; > + cpu_map.socket_count++; > + } > + > + for (unsigned int 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)) > + return true; > + > + 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++; > + } > + return false; > +} > + > int > eal_create_cpu_map(void) > { [...] > + if (eal_check_for_duplicate_numa(info)) > + break; Assignments to "full" are lost, so if we run out of space to store info, users won't get a warning message. > > 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. */ > @@ -139,7 +208,7 @@ eal_create_cpu_map(void) > > free(infos); > > - return 0; > + return ret; > } > > int > @@ -165,3 +234,12 @@ eal_socket_numa_node(unsigned int socket_id) > { > return cpu_map.sockets[socket_id].node_id; > } > + > +PGROUP_AFFINITY > +eal_get_cpu_affinity(size_t cpu_index) > +{ > + if (cpu_index < CPU_SETSIZE) > + return &cpu_map.cpus[cpu_index]; > + > + return NULL; > +} > diff --git a/lib/librte_eal/windows/eal_windows.h b/lib/librte_eal/windows/eal_windows.h > index 478accc1b..dc5dc8240 100644 > --- a/lib/librte_eal/windows/eal_windows.h > +++ b/lib/librte_eal/windows/eal_windows.h > @@ -55,6 +55,16 @@ int eal_thread_create(pthread_t *thread); > */ > unsigned int eal_socket_numa_node(unsigned int socket_id); > > +/** > + * Get pointer to the group affinity for the cpu. > + * > + * @param cpu_index > + * Index of the cpu, as it comes from rte_cpuset_t. > + * @return > + * Pointer to the group affinity for the cpu. > + */ > +PGROUP_AFFINITY eal_get_cpu_affinity(size_t cpu_index); NULL can be returned. Actually, it's a private API always used in a way that cannot trigger error, so probably can be replaced with an assert. > + > /** > * Schedule code for execution in the interrupt thread. > * > diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c > index ecd2f810f..2fa130b1f 100644 > --- a/lib/librte_eal/windows/rte_thread.c > +++ b/lib/librte_eal/windows/rte_thread.c > @@ -4,9 +4,9 @@ > */ > > #include > -#include > #include > -#include > + > +#include "eal_windows.h" > > struct eal_tls_key { > DWORD thread_index; > @@ -69,6 +69,134 @@ rte_thread_equal(rte_thread_t t1, rte_thread_t t2) > return t1 == t2 ? 1 : 0; > } > > +static int > +rte_convert_cpuset_to_affinity(const rte_cpuset_t *cpuset, > + PGROUP_AFFINITY affinity) > +{ > + int ret = 0; > + PGROUP_AFFINITY cpu_affinity = NULL; > + > + 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 (unsigned int 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) { Just to be clear: is it a kernel limitation that a thread can only run on cores of one processor group, or do we impose it so that API is atomic (transactional), i.e. because one of multiple SetThreadGroupAffinity() calls may fail and leave thread partially affinitized? > + ret = EINVAL; > + goto cleanup; > + } > + > + affinity->Mask |= cpu_affinity->Mask; > + } > + > + if (affinity->Mask == 0) { > + ret = EINVAL; > + goto cleanup; > + } > + > +cleanup: This label and `goto`s above are not needed. > + return ret; > +} > + > +int rte_thread_set_affinity_by_id(rte_thread_t thread_id, > + size_t cpuset_size, > + const rte_cpuset_t *cpuset) > +{ Return type should be on a separate line here and in Unix file, too. [...]