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 A9249423C7; Fri, 13 Jan 2023 20:44:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8E86A40E0F; Fri, 13 Jan 2023 20:44:52 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 3B62C40042 for ; Fri, 13 Jan 2023 20:44:51 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 5751120DED6B; Fri, 13 Jan 2023 11:44:50 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5751120DED6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1673639090; bh=4DvLEz2NcCvRgzAdBTAUq/Rfx/GSnxbtyl1DWTAxLNE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=tAsyxA/f28eJN48hTW1IFifZjDZiMU0KLLLYJpe9nDFQS1O32WvJotNz5P5ixWDDn J/b4TvJwMKHfFY0bZjGz1RtrCA+ENFAmnEItqaAIVqK1+AvUflF17hVGEveFCoiX8t PxVAAHRPKmvS31czb8LSrhe+Gm9Z8glh1kiIvZVA= Date: Fri, 13 Jan 2023 11:44:50 -0800 From: Tyler Retzlaff To: dev@dpdk.org Cc: david.marchand@redhat.com, thomas@monjalon.net Subject: Re: [PATCH] eal: replace lcore direct use of pthread in the EAL Message-ID: <20230113194450.GA19372@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1670536096-4090-1-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1670536096-4090-1-git-send-email-roretzla@linux.microsoft.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 hey folks, any feedback here? incremental progress untangling pthread out of eal. thanks! On Thu, Dec 08, 2022 at 01:48:16PM -0800, Tyler Retzlaff wrote: > * Replace the use of pthread_t in struct lcore_config with the EAL > rte_thread_t type. > > * Replace the direct use of pthread_create(), pthread_self() > pthread_getaffinity_np() and pthread_setaffinity_np(). > > Minor tweaks to return value comparisons to align with current DPDK > style. > > Signed-off-by: Tyler Retzlaff > --- > lib/eal/common/eal_common_options.c | 8 ++++---- > lib/eal/common/eal_common_thread.c | 13 ++++++------- > lib/eal/common/eal_private.h | 2 +- > lib/eal/common/eal_thread.h | 2 +- > lib/eal/freebsd/eal.c | 13 ++++++++----- > lib/eal/linux/eal.c | 24 +++++++++++++++++------- > lib/eal/windows/eal.c | 10 ++++++---- > lib/eal/windows/eal_thread.c | 25 ------------------------- > lib/eal/windows/eal_windows.h | 12 ------------ > 9 files changed, 43 insertions(+), 66 deletions(-) > > diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c > index 2d65357..86a519c 100644 > --- a/lib/eal/common/eal_common_options.c > +++ b/lib/eal/common/eal_common_options.c > @@ -1943,8 +1943,8 @@ static int xdigit2val(unsigned char c) > unsigned int removed = 0; > rte_cpuset_t affinity_set; > > - if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > - &affinity_set)) > + if (rte_thread_get_affinity_by_id(rte_thread_self(), > + &affinity_set) != 0) > CPU_ZERO(&affinity_set); > > for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) { > @@ -1972,8 +1972,8 @@ static int xdigit2val(unsigned char c) > } > RTE_CPU_NOT(cpuset, cpuset); > > - if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > - &default_set)) > + if (rte_thread_get_affinity_by_id(rte_thread_self(), > + &default_set) != 0) > CPU_ZERO(&default_set); > > RTE_CPU_AND(cpuset, cpuset, &default_set); > diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c > index c5d8b43..dbe52a4 100644 > --- a/lib/eal/common/eal_common_thread.c > +++ b/lib/eal/common/eal_common_thread.c > @@ -85,9 +85,9 @@ unsigned rte_socket_id(void) > int > rte_thread_set_affinity(rte_cpuset_t *cpusetp) > { > - if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > + if (rte_thread_set_affinity_by_id(rte_thread_self(), > cpusetp) != 0) { > - RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n"); > + RTE_LOG(ERR, EAL, "rte_thread_set_affinity_by_id failed\n"); > return -1; > } > > @@ -166,7 +166,7 @@ unsigned rte_socket_id(void) > } > > /* main loop of threads */ > -__rte_noreturn void * > +__rte_noreturn uint32_t > eal_thread_loop(void *arg) > { > unsigned int lcore_id = (uintptr_t)arg; > @@ -223,8 +223,7 @@ unsigned rte_socket_id(void) > } > > /* never reached */ > - /* pthread_exit(NULL); */ > - /* return NULL; */ > + /* return 0; */ > } > > enum __rte_ctrl_thread_status { > @@ -253,7 +252,7 @@ static void *ctrl_thread_init(void *arg) > void *routine_arg = params->arg; > > __rte_thread_init(rte_lcore_id(), cpuset); > - params->ret = pthread_setaffinity_np(pthread_self(), sizeof(*cpuset), > + params->ret = rte_thread_set_affinity_by_id(rte_thread_self(), > cpuset); > if (params->ret != 0) { > __atomic_store_n(¶ms->ctrl_thread_status, > @@ -338,7 +337,7 @@ static void *ctrl_thread_init(void *arg) > rte_errno = EINVAL; > return -1; > } > - if (pthread_getaffinity_np(pthread_self(), sizeof(cpuset), > + if (rte_thread_get_affinity_by_id(rte_thread_self(), > &cpuset) != 0) > CPU_ZERO(&cpuset); > lcore_id = eal_lcore_non_eal_allocate(); > diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h > index 0f4d75b..d7f8377 100644 > --- a/lib/eal/common/eal_private.h > +++ b/lib/eal/common/eal_private.h > @@ -20,7 +20,7 @@ > * Structure storing internal configuration (per-lcore) > */ > struct lcore_config { > - pthread_t thread_id; /**< pthread identifier */ > + rte_thread_t thread_id; /**< thread identifier */ > int pipe_main2worker[2]; /**< communication pipe with main */ > int pipe_worker2main[2]; /**< communication pipe with main */ > > diff --git a/lib/eal/common/eal_thread.h b/lib/eal/common/eal_thread.h > index 32bfe5c..1c3c344 100644 > --- a/lib/eal/common/eal_thread.h > +++ b/lib/eal/common/eal_thread.h > @@ -14,7 +14,7 @@ > * @param arg > * The lcore_id (passed as an integer) of this worker thread. > */ > -__rte_noreturn void *eal_thread_loop(void *arg); > +__rte_noreturn uint32_t eal_thread_loop(void *arg); > > /** > * Get the NUMA socket id from cpu id. > diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c > index 607684c..1deee2c 100644 > --- a/lib/eal/freebsd/eal.c > +++ b/lib/eal/freebsd/eal.c > @@ -780,7 +780,7 @@ static void rte_eal_init_alert(const char *msg) > > eal_check_mem_on_local_socket(); > > - if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > + if (rte_thread_set_affinity_by_id(rte_thread_self(), > &lcore_config[config->main_lcore].cpuset) != 0) { > rte_eal_init_alert("Cannot set affinity"); > rte_errno = EINVAL; > @@ -809,7 +809,7 @@ static void rte_eal_init_alert(const char *msg) > lcore_config[i].state = WAIT; > > /* create a thread for each lcore */ > - ret = pthread_create(&lcore_config[i].thread_id, NULL, > + ret = rte_thread_create(&lcore_config[i].thread_id, NULL, > eal_thread_loop, (void *)(uintptr_t)i); > if (ret != 0) > rte_panic("Cannot create thread\n"); > @@ -817,10 +817,13 @@ static void rte_eal_init_alert(const char *msg) > /* Set thread_name for aid in debugging. */ > snprintf(thread_name, sizeof(thread_name), > "rte-worker-%d", i); > - rte_thread_setname(lcore_config[i].thread_id, thread_name); > + rte_thread_setname( > + (pthread_t)lcore_config[i].thread_id.opaque_id, > + thread_name); > > - ret = pthread_setaffinity_np(lcore_config[i].thread_id, > - sizeof(rte_cpuset_t), &lcore_config[i].cpuset); > + ret = rte_thread_set_affinity_by_id( > + lcore_config[i].thread_id, > + &lcore_config[i].cpuset); > if (ret != 0) > rte_panic("Cannot set affinity\n"); > } > diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c > index 8c118d0..edef77d 100644 > --- a/lib/eal/linux/eal.c > +++ b/lib/eal/linux/eal.c > @@ -909,6 +909,13 @@ static void rte_eal_init_alert(const char *msg) > return n > 2; > } > > +static > +__rte_noreturn void * > +eal_worker_thread_loop(void *arg) > +{ > + eal_thread_loop(arg); > +} > + > static int > eal_worker_thread_create(unsigned int lcore_id) > { > @@ -943,8 +950,9 @@ static void rte_eal_init_alert(const char *msg) > } > } > > - if (pthread_create(&lcore_config[lcore_id].thread_id, attrp, > - eal_thread_loop, (void *)(uintptr_t)lcore_id) == 0) > + if (pthread_create( > + (pthread_t *)&lcore_config[lcore_id].thread_id.opaque_id, > + attrp, eal_worker_thread_loop, (void *)(uintptr_t)lcore_id) == 0) > ret = 0; > > out: > @@ -1220,7 +1228,7 @@ static void rte_eal_init_alert(const char *msg) > > eal_check_mem_on_local_socket(); > > - if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > + if (rte_thread_set_affinity_by_id(rte_thread_self(), > &lcore_config[config->main_lcore].cpuset) != 0) { > rte_eal_init_alert("Cannot set affinity"); > rte_errno = EINVAL; > @@ -1255,14 +1263,16 @@ static void rte_eal_init_alert(const char *msg) > /* Set thread_name for aid in debugging. */ > snprintf(thread_name, sizeof(thread_name), > "rte-worker-%d", i); > - ret = rte_thread_setname(lcore_config[i].thread_id, > - thread_name); > + ret = rte_thread_setname( > + (pthread_t)lcore_config[i].thread_id.opaque_id, > + thread_name); > if (ret != 0) > RTE_LOG(DEBUG, EAL, > "Cannot set name for lcore thread\n"); > > - ret = pthread_setaffinity_np(lcore_config[i].thread_id, > - sizeof(rte_cpuset_t), &lcore_config[i].cpuset); > + ret = rte_thread_set_affinity_by_id( > + lcore_config[i].thread_id, > + &lcore_config[i].cpuset); > if (ret != 0) > rte_panic("Cannot set affinity\n"); > } > diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c > index adb929a..a95cbc5 100644 > --- a/lib/eal/windows/eal.c > +++ b/lib/eal/windows/eal.c > @@ -404,7 +404,7 @@ enum rte_proc_type_t > return -1; > } > > - if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t), > + if (rte_thread_set_affinity_by_id(rte_thread_self(), > &lcore_config[config->main_lcore].cpuset) != 0) { > rte_eal_init_alert("Cannot set affinity"); > rte_errno = EINVAL; > @@ -434,10 +434,12 @@ enum rte_proc_type_t > lcore_config[i].state = WAIT; > > /* create a thread for each lcore */ > - if (eal_thread_create(&lcore_config[i].thread_id, i) != 0) > + if (rte_thread_create(&lcore_config[i].thread_id, NULL, > + eal_thread_loop, (void *)(uintptr_t)i) != 0) > rte_panic("Cannot create thread\n"); > - ret = pthread_setaffinity_np(lcore_config[i].thread_id, > - sizeof(rte_cpuset_t), &lcore_config[i].cpuset); > + ret = rte_thread_set_affinity_by_id( > + lcore_config[i].thread_id, > + &lcore_config[i].cpuset); > if (ret != 0) > RTE_LOG(DEBUG, EAL, "Cannot set affinity\n"); > } > diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c > index caffe68..464d510 100644 > --- a/lib/eal/windows/eal_thread.c > +++ b/lib/eal/windows/eal_thread.c > @@ -70,31 +70,6 @@ > rte_panic("cannot write on configuration pipe\n"); > } > > -/* function to create threads */ > -int > -eal_thread_create(pthread_t *thread, unsigned int lcore_id) > -{ > - HANDLE th; > - > - th = CreateThread(NULL, 0, > - (LPTHREAD_START_ROUTINE)(ULONG_PTR)eal_thread_loop, > - (LPVOID)(uintptr_t)lcore_id, > - CREATE_SUSPENDED, > - (LPDWORD)thread); > - if (!th) > - return -1; > - > - SetPriorityClass(GetCurrentProcess(), NORMAL_PRIORITY_CLASS); > - SetThreadPriority(th, THREAD_PRIORITY_NORMAL); > - > - if (ResumeThread(th) == (DWORD)-1) { > - (void)CloseHandle(th); > - return -1; > - } > - > - return 0; > -} > - > /* get current thread ID */ > int > rte_sys_gettid(void) > diff --git a/lib/eal/windows/eal_windows.h b/lib/eal/windows/eal_windows.h > index ab25814..43b228d 100644 > --- a/lib/eal/windows/eal_windows.h > +++ b/lib/eal/windows/eal_windows.h > @@ -36,18 +36,6 @@ > int eal_create_cpu_map(void); > > /** > - * Create a thread. > - * > - * @param thread > - * The location to store the thread id if successful. > - * @param lcore_id > - * The lcore_id of this worker thread. > - * @return > - * 0 for success, -1 if the thread is not created. > - */ > -int eal_thread_create(pthread_t *thread, unsigned int lcore_id); > - > -/** > * Get system NUMA node number for a socket ID. > * > * @param socket_id > -- > 1.8.3.1