From patchwork Fri Apr 1 08:44:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 109064 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9A747A0503; Fri, 1 Apr 2022 10:44:41 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3C3D94067E; Fri, 1 Apr 2022 10:44:41 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 1DDEA4014F for ; Fri, 1 Apr 2022 10:44:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648802678; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2hYKblygkmv+J96E2yaKOonj8z+apdTI1+tKw3h58AY=; b=iClUztND5CXdCW0fYZGwPfNbiPCXtBAYbDI1G0zKUzyyzknyjuwjSLcQ+Xtj1556sDj05s QrisltJ7jBifbSSJ3EyS0PFASo7ax/7tdf5S47nQ+z0pdLEBAa0HOKg795cXpSBbuvIjJ1 qDwkVyUev3PRkDrOp8SxW6p+PScqIjc= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-176--7ut9uddP9elGEOSSu5p3A-1; Fri, 01 Apr 2022 04:44:33 -0400 X-MC-Unique: -7ut9uddP9elGEOSSu5p3A-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id F0AEB185A794; Fri, 1 Apr 2022 08:44:32 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 77AEA401DBC; Fri, 1 Apr 2022 08:44:31 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: thomas@monjalon.net, Bruce Richardson , Dmitry Kozlyuk , Narcisa Ana Maria Vasile , Dmitry Malloy , Pallavi Kadam Subject: [PATCH v2 1/2] eal: cleanup lcore ID hand-over Date: Fri, 1 Apr 2022 10:44:18 +0200 Message-Id: <20220401084419.21150-1-david.marchand@redhat.com> In-Reply-To: <20220323093001.20618-1-david.marchand@redhat.com> References: <20220323093001.20618-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 So far, a worker thread has been using its thread_id to discover which lcore has been assigned to it. On the other hand, as noted by Tyler, the pthread API does not strictly guarantee that a new thread won't start running eal_thread_loop before pthread_create writes to &lcore_config[xx].thread_id. Though all OS implementations supported in DPDK (recently) ensure this property, it is more robust to have the main thread directly pass the worker thread lcore. Signed-off-by: David Marchand --- lib/eal/common/eal_thread.h | 4 ++-- lib/eal/freebsd/eal.c | 2 +- lib/eal/freebsd/eal_thread.c | 16 +++------------- lib/eal/linux/eal.c | 2 +- lib/eal/linux/eal_thread.c | 17 ++++------------- lib/eal/windows/eal.c | 2 +- lib/eal/windows/eal_thread.c | 23 +++++++---------------- 7 files changed, 19 insertions(+), 47 deletions(-) diff --git a/lib/eal/common/eal_thread.h b/lib/eal/common/eal_thread.h index 4a49117be8..3c84efd553 100644 --- a/lib/eal/common/eal_thread.h +++ b/lib/eal/common/eal_thread.h @@ -8,10 +8,10 @@ #include /** - * basic loop of thread, called for each thread by eal_init(). + * Basic loop of EAL thread, called for each worker thread by rte_eal_init(). * * @param arg - * opaque pointer + * The lcore_id (passed as an integer) of this worker thread. */ __rte_noreturn void *eal_thread_loop(void *arg); diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c index 71993fe25b..e233e57184 100644 --- a/lib/eal/freebsd/eal.c +++ b/lib/eal/freebsd/eal.c @@ -813,7 +813,7 @@ rte_eal_init(int argc, char **argv) /* create a thread for each lcore */ ret = pthread_create(&lcore_config[i].thread_id, NULL, - eal_thread_loop, NULL); + eal_thread_loop, (void *)(uintptr_t)i); if (ret != 0) rte_panic("Cannot create thread\n"); diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c index 3b18030d73..9044d70ef7 100644 --- a/lib/eal/freebsd/eal_thread.c +++ b/lib/eal/freebsd/eal_thread.c @@ -76,25 +76,15 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id) /* main loop of threads */ __rte_noreturn void * -eal_thread_loop(__rte_unused void *arg) +eal_thread_loop(void *arg) { + unsigned int lcore_id = (uintptr_t)arg; char c; int n, ret; unsigned lcore_id; - pthread_t thread_id; int m2w, w2m; char cpuset[RTE_CPU_AFFINITY_STR_LEN]; - thread_id = pthread_self(); - - /* retrieve our lcore_id from the configuration structure */ - RTE_LCORE_FOREACH_WORKER(lcore_id) { - if (thread_id == lcore_config[lcore_id].thread_id) - break; - } - if (lcore_id == RTE_MAX_LCORE) - rte_panic("cannot retrieve lcore id\n"); - m2w = lcore_config[lcore_id].pipe_main2worker[0]; w2m = lcore_config[lcore_id].pipe_worker2main[1]; @@ -102,7 +92,7 @@ eal_thread_loop(__rte_unused void *arg) ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset)); RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n", - lcore_id, thread_id, cpuset, ret == 0 ? "" : "..."); + lcore_id, pthread_self(), cpuset, ret == 0 ? "" : "..."); rte_eal_trace_thread_lcore_ready(lcore_id, cpuset); diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c index 025e5cc10d..98c6838026 100644 --- a/lib/eal/linux/eal.c +++ b/lib/eal/linux/eal.c @@ -1147,7 +1147,7 @@ rte_eal_init(int argc, char **argv) /* create a thread for each lcore */ ret = pthread_create(&lcore_config[i].thread_id, NULL, - eal_thread_loop, NULL); + eal_thread_loop, (void *)(uintptr_t)i); if (ret != 0) rte_panic("Cannot create thread\n"); diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c index fa6cd7e2c4..26b0a7d48a 100644 --- a/lib/eal/linux/eal_thread.c +++ b/lib/eal/linux/eal_thread.c @@ -70,24 +70,14 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id) /* main loop of threads */ __rte_noreturn void * -eal_thread_loop(__rte_unused void *arg) +eal_thread_loop(void *arg) { + unsigned int lcore_id = (uintptr_t)arg; char c; int n, ret; - unsigned lcore_id; - pthread_t thread_id; int m2w, w2m; char cpuset[RTE_CPU_AFFINITY_STR_LEN]; - thread_id = pthread_self(); - - /* retrieve our lcore_id from the configuration structure */ - RTE_LCORE_FOREACH_WORKER(lcore_id) { - if (thread_id == lcore_config[lcore_id].thread_id) - break; - } - if (lcore_id == RTE_MAX_LCORE) - rte_panic("cannot retrieve lcore id\n"); m2w = lcore_config[lcore_id].pipe_main2worker[0]; w2m = lcore_config[lcore_id].pipe_worker2main[1]; @@ -96,7 +86,8 @@ eal_thread_loop(__rte_unused void *arg) ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset)); RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s%s])\n", - lcore_id, (uintptr_t)thread_id, cpuset, ret == 0 ? "" : "..."); + lcore_id, (uintptr_t)pthread_self(), cpuset, + ret == 0 ? "" : "..."); rte_eal_trace_thread_lcore_ready(lcore_id, cpuset); diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c index ca3c41aaa7..1874f9f6d7 100644 --- a/lib/eal/windows/eal.c +++ b/lib/eal/windows/eal.c @@ -420,7 +420,7 @@ rte_eal_init(int argc, char **argv) lcore_config[i].state = WAIT; /* create a thread for each lcore */ - if (eal_thread_create(&lcore_config[i].thread_id) != 0) + if (eal_thread_create(&lcore_config[i].thread_id, 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); diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c index ff84cb42af..95e96548de 100644 --- a/lib/eal/windows/eal_thread.c +++ b/lib/eal/windows/eal_thread.c @@ -63,32 +63,21 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id) /* main loop of threads */ void * -eal_thread_loop(void *arg __rte_unused) +eal_thread_loop(void *arg) { + unsigned int lcore_id = (uintptr_t)arg; char c; int n, ret; - unsigned int lcore_id; - pthread_t thread_id; int m2w, w2m; char cpuset[RTE_CPU_AFFINITY_STR_LEN]; - thread_id = pthread_self(); - - /* retrieve our lcore_id from the configuration structure */ - RTE_LCORE_FOREACH_WORKER(lcore_id) { - if (thread_id == lcore_config[lcore_id].thread_id) - break; - } - if (lcore_id == RTE_MAX_LCORE) - rte_panic("cannot retrieve lcore id\n"); - m2w = lcore_config[lcore_id].pipe_main2worker[0]; w2m = lcore_config[lcore_id].pipe_worker2main[1]; __rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset); RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s])\n", - lcore_id, (uintptr_t)thread_id, cpuset); + lcore_id, (uintptr_t)pthread_self(), cpuset); /* read on our pipe to get commands */ while (1) { @@ -144,13 +133,15 @@ eal_thread_loop(void *arg __rte_unused) /* function to create threads */ int -eal_thread_create(pthread_t *thread) +eal_thread_create(pthread_t *thread, unsigned int lcore_id) { HANDLE th; th = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)(ULONG_PTR)eal_thread_loop, - NULL, CREATE_SUSPENDED, (LPDWORD)thread); + (LPVOID)(uintptr_t)lcore_id, + CREATE_SUSPENDED, + (LPDWORD)thread); if (!th) return -1;