From patchwork Mon Oct 25 04:52:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 102725 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 38A84A0C45; Mon, 25 Oct 2021 06:53:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5AD8F410F8; Mon, 25 Oct 2021 06:53:02 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 4EB474111C; Mon, 25 Oct 2021 06:53:00 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C23411FB; Sun, 24 Oct 2021 21:52:59 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.12.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B97D83F73D; Sun, 24 Oct 2021 21:52:59 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, honnappa.nagarahalli@arm.com, konstantin.ananyev@intel.com, david.marchand@redhat.com, feifei.wang2@arm.com Cc: ruifeng.wang@arm.com, nd@arm.com, stable@dpdk.org Date: Sun, 24 Oct 2021 23:52:34 -0500 Message-Id: <20211025045237.19878-2-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211025045237.19878-1-honnappa.nagarahalli@arm.com> References: <20210224212018.17576-1-honnappa.nagarahalli@arm.com> <20211025045237.19878-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 1/4] eal: reset lcore function pointer and argument 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" In the rte_eal_remote_launch function, the lcore function pointer is checked for NULL. However, the pointer is never reset to NULL. Reset the lcore function pointer and argument after the worker has completed executing the lcore function. Fixes: af75078fece3 ("first public release") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli Reviewed-by: Ruifeng Wang Reviewed-by: Feifei Wang --- lib/eal/freebsd/eal_thread.c | 2 ++ lib/eal/linux/eal_thread.c | 2 ++ lib/eal/windows/eal_thread.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c index 1dce9b04f2..bbc3a8e985 100644 --- a/lib/eal/freebsd/eal_thread.c +++ b/lib/eal/freebsd/eal_thread.c @@ -126,6 +126,8 @@ eal_thread_loop(__rte_unused void *arg) fct_arg = lcore_config[lcore_id].arg; ret = lcore_config[lcore_id].f(fct_arg); lcore_config[lcore_id].ret = ret; + lcore_config[lcore_id].f = NULL; + lcore_config[lcore_id].arg = NULL; rte_wmb(); lcore_config[lcore_id].state = FINISHED; } diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c index 83c2034b93..8f3c0dafd6 100644 --- a/lib/eal/linux/eal_thread.c +++ b/lib/eal/linux/eal_thread.c @@ -126,6 +126,8 @@ eal_thread_loop(__rte_unused void *arg) fct_arg = lcore_config[lcore_id].arg; ret = lcore_config[lcore_id].f(fct_arg); lcore_config[lcore_id].ret = ret; + lcore_config[lcore_id].f = NULL; + lcore_config[lcore_id].arg = NULL; rte_wmb(); /* when a service core returns, it should go directly to WAIT diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c index 9c3f6d69fd..df1df5d02c 100644 --- a/lib/eal/windows/eal_thread.c +++ b/lib/eal/windows/eal_thread.c @@ -110,6 +110,8 @@ eal_thread_loop(void *arg __rte_unused) fct_arg = lcore_config[lcore_id].arg; ret = lcore_config[lcore_id].f(fct_arg); lcore_config[lcore_id].ret = ret; + lcore_config[lcore_id].f = NULL; + lcore_config[lcore_id].arg = NULL; rte_wmb(); /* when a service core returns, it should go directly to WAIT From patchwork Mon Oct 25 04:52:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 102726 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 D81B1A0C45; Mon, 25 Oct 2021 06:53:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9F96641137; Mon, 25 Oct 2021 06:53:04 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 4C6B5410EB for ; Mon, 25 Oct 2021 06:53:01 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B91E41063; Sun, 24 Oct 2021 21:53:00 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.12.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A4AC53F73D; Sun, 24 Oct 2021 21:53:00 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, honnappa.nagarahalli@arm.com, konstantin.ananyev@intel.com, david.marchand@redhat.com, feifei.wang2@arm.com Cc: ruifeng.wang@arm.com, nd@arm.com Date: Sun, 24 Oct 2021 23:52:35 -0500 Message-Id: <20211025045237.19878-3-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211025045237.19878-1-honnappa.nagarahalli@arm.com> References: <20210224212018.17576-1-honnappa.nagarahalli@arm.com> <20211025045237.19878-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 2/4] eal: lcore state FINISHED is not required 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" FINISHED state seems to be used to indicate that the worker's update of the 'state' is not visible to other threads. There seems to be no requirement to have such a state. Since the FINISHED state is removed, the API rte_eal_wait_lcore is updated to always return the status of the last function that ran in the worker core. Signed-off-by: Honnappa Nagarahalli Reviewed-by: Ola Liljedahl Reviewed-by: Feifei Wang --- drivers/event/dpaa2/dpaa2_eventdev_selftest.c | 2 +- drivers/event/octeontx/ssovf_evdev_selftest.c | 2 +- drivers/event/sw/sw_evdev_selftest.c | 4 ++-- examples/l2fwd-keepalive/main.c | 3 +-- lib/eal/common/eal_common_launch.c | 7 ++----- lib/eal/freebsd/eal_thread.c | 4 ++-- lib/eal/include/rte_launch.h | 21 +++++++++---------- lib/eal/include/rte_service.h | 4 +--- lib/eal/linux/eal_thread.c | 10 ++------- lib/eal/windows/eal_thread.c | 10 ++------- 10 files changed, 24 insertions(+), 43 deletions(-) diff --git a/drivers/event/dpaa2/dpaa2_eventdev_selftest.c b/drivers/event/dpaa2/dpaa2_eventdev_selftest.c index cd7311a94d..bbbd20951f 100644 --- a/drivers/event/dpaa2/dpaa2_eventdev_selftest.c +++ b/drivers/event/dpaa2/dpaa2_eventdev_selftest.c @@ -468,7 +468,7 @@ wait_workers_to_join(int lcore, const rte_atomic32_t *count) RTE_SET_USED(count); print_cycles = cycles = rte_get_timer_cycles(); - while (rte_eal_get_lcore_state(lcore) != FINISHED) { + while (rte_eal_get_lcore_state(lcore) != WAIT) { uint64_t new_cycles = rte_get_timer_cycles(); if (new_cycles - print_cycles > rte_get_timer_hz()) { diff --git a/drivers/event/octeontx/ssovf_evdev_selftest.c b/drivers/event/octeontx/ssovf_evdev_selftest.c index 528f99dd84..d7b0d22111 100644 --- a/drivers/event/octeontx/ssovf_evdev_selftest.c +++ b/drivers/event/octeontx/ssovf_evdev_selftest.c @@ -579,7 +579,7 @@ wait_workers_to_join(int lcore, const rte_atomic32_t *count) RTE_SET_USED(count); print_cycles = cycles = rte_get_timer_cycles(); - while (rte_eal_get_lcore_state(lcore) != FINISHED) { + while (rte_eal_get_lcore_state(lcore) != WAIT) { uint64_t new_cycles = rte_get_timer_cycles(); if (new_cycles - print_cycles > rte_get_timer_hz()) { diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c index d53e903129..9768d3a0c7 100644 --- a/drivers/event/sw/sw_evdev_selftest.c +++ b/drivers/event/sw/sw_evdev_selftest.c @@ -3140,8 +3140,8 @@ worker_loopback(struct test *t, uint8_t disable_implicit_release) rte_eal_remote_launch(worker_loopback_worker_fn, t, w_lcore); print_cycles = cycles = rte_get_timer_cycles(); - while (rte_eal_get_lcore_state(p_lcore) != FINISHED || - rte_eal_get_lcore_state(w_lcore) != FINISHED) { + while (rte_eal_get_lcore_state(p_lcore) != WAIT || + rte_eal_get_lcore_state(w_lcore) != WAIT) { rte_service_run_iter_on_app_lcore(t->service_id, 1); diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c index 4e1a17cfe4..b4f9d7334b 100644 --- a/examples/l2fwd-keepalive/main.c +++ b/examples/l2fwd-keepalive/main.c @@ -507,8 +507,7 @@ dead_core(__rte_unused void *ptr_data, const int id_core) if (terminate_signal_received) return; printf("Dead core %i - restarting..\n", id_core); - if (rte_eal_get_lcore_state(id_core) == FINISHED) { - rte_eal_wait_lcore(id_core); + if (rte_eal_get_lcore_state(id_core) == WAIT) { rte_eal_remote_launch(l2fwd_launch_one_lcore, NULL, id_core); } else { printf("..false positive!\n"); diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c index 34f854ad80..78fd940267 100644 --- a/lib/eal/common/eal_common_launch.c +++ b/lib/eal/common/eal_common_launch.c @@ -26,14 +26,11 @@ rte_eal_wait_lcore(unsigned worker_id) if (lcore_config[worker_id].state == WAIT) return 0; - while (lcore_config[worker_id].state != WAIT && - lcore_config[worker_id].state != FINISHED) + while (lcore_config[worker_id].state != WAIT) rte_pause(); rte_rmb(); - /* we are in finished state, go to wait state */ - lcore_config[worker_id].state = WAIT; return lcore_config[worker_id].ret; } @@ -62,7 +59,7 @@ rte_eal_mp_remote_launch(int (*f)(void *), void *arg, if (call_main == CALL_MAIN) { lcore_config[main_lcore].ret = f(arg); - lcore_config[main_lcore].state = FINISHED; + lcore_config[main_lcore].state = WAIT; } return 0; diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c index bbc3a8e985..ce314c1dd4 100644 --- a/lib/eal/freebsd/eal_thread.c +++ b/lib/eal/freebsd/eal_thread.c @@ -28,7 +28,7 @@ /* * Send a message to a worker lcore identified by worker_id to call a * function f with argument arg. Once the execution is done, the - * remote lcore switch in FINISHED state. + * remote lcore switches to WAIT state. */ int rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id) @@ -129,7 +129,7 @@ eal_thread_loop(__rte_unused void *arg) lcore_config[lcore_id].f = NULL; lcore_config[lcore_id].arg = NULL; rte_wmb(); - lcore_config[lcore_id].state = FINISHED; + lcore_config[lcore_id].state = WAIT; } /* never reached */ diff --git a/lib/eal/include/rte_launch.h b/lib/eal/include/rte_launch.h index 22a901ce62..f2d386e6e2 100644 --- a/lib/eal/include/rte_launch.h +++ b/lib/eal/include/rte_launch.h @@ -19,9 +19,10 @@ extern "C" { * State of an lcore. */ enum rte_lcore_state_t { - WAIT, /**< waiting a new command */ - RUNNING, /**< executing command */ - FINISHED, /**< command executed */ + WAIT, + /**< waiting for new command */ + RUNNING, + /**< executing command */ }; /** @@ -41,7 +42,7 @@ typedef int (lcore_function_t)(void *); * * When the remote lcore receives the message, it switches to * the RUNNING state, then calls the function f with argument arg. Once the - * execution is done, the remote lcore switches to a FINISHED state and + * execution is done, the remote lcore switches to WAIT state and * the return value of f is stored in a local variable to be read using * rte_eal_wait_lcore(). * @@ -118,18 +119,16 @@ enum rte_lcore_state_t rte_eal_get_lcore_state(unsigned int worker_id); * * To be executed on the MAIN lcore only. * - * If the worker lcore identified by the worker_id is in a FINISHED state, - * switch to the WAIT state. If the lcore is in RUNNING state, wait until - * the lcore finishes its job and moves to the FINISHED state. + * If the lcore identified by the worker_id is in RUNNING state, wait until + * the lcore finishes its job and moves to the WAIT state. * * @param worker_id * The identifier of the lcore. * @return - * - 0: If the lcore identified by the worker_id is in a WAIT state. + * - 0: If the remote launch function was never called on the lcore + * identified by the worker_id. * - The value that was returned by the previous remote launch - * function call if the lcore identified by the worker_id was in a - * FINISHED or RUNNING state. In this case, it changes the state - * of the lcore to WAIT. + * function call. */ int rte_eal_wait_lcore(unsigned worker_id); diff --git a/lib/eal/include/rte_service.h b/lib/eal/include/rte_service.h index c7d037d862..646533e871 100644 --- a/lib/eal/include/rte_service.h +++ b/lib/eal/include/rte_service.h @@ -321,9 +321,7 @@ int32_t rte_service_lcore_count(void); * from duty, just unmaps all services / cores, and stops() the service cores. * The runstate of services is not modified. * - * The cores that are stopped with this call, are in FINISHED state and - * the application must take care of bringing them back to a launchable state: - * e.g. call *rte_eal_lcore_wait* on the lcore_id. + * The cores that are stopped with this call, are in WAIT state. * * @retval 0 Success */ diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c index 8f3c0dafd6..e2c9e5a3f7 100644 --- a/lib/eal/linux/eal_thread.c +++ b/lib/eal/linux/eal_thread.c @@ -28,7 +28,7 @@ /* * Send a message to a worker lcore identified by worker_id to call a * function f with argument arg. Once the execution is done, the - * remote lcore switch in FINISHED state. + * remote lcore switches to WAIT state. */ int rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id) @@ -130,13 +130,7 @@ eal_thread_loop(__rte_unused void *arg) lcore_config[lcore_id].arg = NULL; rte_wmb(); - /* when a service core returns, it should go directly to WAIT - * state, because the application will not lcore_wait() for it. - */ - if (lcore_config[lcore_id].core_role == ROLE_SERVICE) - lcore_config[lcore_id].state = WAIT; - else - lcore_config[lcore_id].state = FINISHED; + lcore_config[lcore_id].state = WAIT; } /* never reached */ diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c index df1df5d02c..0028123c95 100644 --- a/lib/eal/windows/eal_thread.c +++ b/lib/eal/windows/eal_thread.c @@ -19,7 +19,7 @@ /* * Send a message to a worker lcore identified by worker_id to call a * function f with argument arg. Once the execution is done, the - * remote lcore switch in FINISHED state. + * remote lcore switches to WAIT state. */ int rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id) @@ -114,13 +114,7 @@ eal_thread_loop(void *arg __rte_unused) lcore_config[lcore_id].arg = NULL; rte_wmb(); - /* when a service core returns, it should go directly to WAIT - * state, because the application will not lcore_wait() for it. - */ - if (lcore_config[lcore_id].core_role == ROLE_SERVICE) - lcore_config[lcore_id].state = WAIT; - else - lcore_config[lcore_id].state = FINISHED; + lcore_config[lcore_id].state = WAIT; } } From patchwork Mon Oct 25 04:52:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 102727 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 478B3A0C45; Mon, 25 Oct 2021 06:53:17 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ACB2541130; Mon, 25 Oct 2021 06:53:05 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 9244841120 for ; Mon, 25 Oct 2021 06:53:02 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0D4871FB; Sun, 24 Oct 2021 21:53:02 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.12.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id ECB043F73D; Sun, 24 Oct 2021 21:53:01 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, honnappa.nagarahalli@arm.com, konstantin.ananyev@intel.com, david.marchand@redhat.com, feifei.wang2@arm.com Cc: ruifeng.wang@arm.com, nd@arm.com Date: Sun, 24 Oct 2021 23:52:36 -0500 Message-Id: <20211025045237.19878-4-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211025045237.19878-1-honnappa.nagarahalli@arm.com> References: <20210224212018.17576-1-honnappa.nagarahalli@arm.com> <20211025045237.19878-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 3/4] eal: use correct memory ordering 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" Ensure that the memory operations before the call to rte_eal_remote_launch are visible to the worker thread. Use the function pointer to execute in worker thread as the guard variable. Ensure that the memory operations in worker thread, that happen before it returns the status of the assigned function, are visible to the main thread. Use the variable containing the lcore's state as the guard variable. Signed-off-by: Honnappa Nagarahalli Reviewed-by: Ola Liljedahl Reviewed-by: Feifei Wang --- lib/eal/common/eal_common_launch.c | 8 ++---- lib/eal/freebsd/eal_thread.c | 41 ++++++++++++++++++++++++------ lib/eal/linux/eal_thread.c | 40 +++++++++++++++++++++++------ lib/eal/windows/eal_thread.c | 40 +++++++++++++++++++++++------ 4 files changed, 99 insertions(+), 30 deletions(-) diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c index 78fd940267..e95dadffb3 100644 --- a/lib/eal/common/eal_common_launch.c +++ b/lib/eal/common/eal_common_launch.c @@ -23,14 +23,10 @@ int rte_eal_wait_lcore(unsigned worker_id) { - if (lcore_config[worker_id].state == WAIT) - return 0; - - while (lcore_config[worker_id].state != WAIT) + while (__atomic_load_n(&lcore_config[worker_id].state, + __ATOMIC_ACQUIRE) != WAIT) rte_pause(); - rte_rmb(); - return lcore_config[worker_id].ret; } diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c index ce314c1dd4..3b18030d73 100644 --- a/lib/eal/freebsd/eal_thread.c +++ b/lib/eal/freebsd/eal_thread.c @@ -39,11 +39,19 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id) int w2m = lcore_config[worker_id].pipe_worker2main[0]; int rc = -EBUSY; - if (lcore_config[worker_id].state != WAIT) + /* Check if the worker is in 'WAIT' state. Use acquire order + * since 'state' variable is used as the guard variable. + */ + if (__atomic_load_n(&lcore_config[worker_id].state, + __ATOMIC_ACQUIRE) != WAIT) goto finish; - lcore_config[worker_id].f = f; lcore_config[worker_id].arg = arg; + /* Ensure that all the memory operations are completed + * before the worker thread starts running the function. + * Use worker thread function as the guard variable. + */ + __atomic_store_n(&lcore_config[worker_id].f, f, __ATOMIC_RELEASE); /* send message */ n = 0; @@ -100,6 +108,7 @@ eal_thread_loop(__rte_unused void *arg) /* read on our pipe to get commands */ while (1) { + lcore_function_t *f; void *fct_arg; /* wait command */ @@ -110,7 +119,11 @@ eal_thread_loop(__rte_unused void *arg) if (n <= 0) rte_panic("cannot read on configuration pipe\n"); - lcore_config[lcore_id].state = RUNNING; + /* Set the state to 'RUNNING'. Use release order + * since 'state' variable is used as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, RUNNING, + __ATOMIC_RELEASE); /* send ack */ n = 0; @@ -119,17 +132,29 @@ eal_thread_loop(__rte_unused void *arg) if (n < 0) rte_panic("cannot write on configuration pipe\n"); - if (lcore_config[lcore_id].f == NULL) - rte_panic("NULL function pointer\n"); + /* Load 'f' with acquire order to ensure that + * the memory operations from the main thread + * are accessed only after update to 'f' is visible. + * Wait till the update to 'f' is visible to the worker. + */ + while ((f = __atomic_load_n(&lcore_config[lcore_id].f, + __ATOMIC_ACQUIRE)) == NULL) + rte_pause(); /* call the function and store the return value */ fct_arg = lcore_config[lcore_id].arg; - ret = lcore_config[lcore_id].f(fct_arg); + ret = f(fct_arg); lcore_config[lcore_id].ret = ret; lcore_config[lcore_id].f = NULL; lcore_config[lcore_id].arg = NULL; - rte_wmb(); - lcore_config[lcore_id].state = WAIT; + + /* Store the state with release order to ensure that + * the memory operations from the worker thread + * are completed before the state is updated. + * Use 'state' as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, WAIT, + __ATOMIC_RELEASE); } /* never reached */ diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c index e2c9e5a3f7..c7f0f9b2f7 100644 --- a/lib/eal/linux/eal_thread.c +++ b/lib/eal/linux/eal_thread.c @@ -39,11 +39,19 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id) int w2m = lcore_config[worker_id].pipe_worker2main[0]; int rc = -EBUSY; - if (lcore_config[worker_id].state != WAIT) + /* Check if the worker is in 'WAIT' state. Use acquire order + * since 'state' variable is used as the guard variable. + */ + if (__atomic_load_n(&lcore_config[worker_id].state, + __ATOMIC_ACQUIRE) != WAIT) goto finish; - lcore_config[worker_id].f = f; lcore_config[worker_id].arg = arg; + /* Ensure that all the memory operations are completed + * before the worker thread starts running the function. + * Use worker thread function pointer as the guard variable. + */ + __atomic_store_n(&lcore_config[worker_id].f, f, __ATOMIC_RELEASE); /* send message */ n = 0; @@ -100,6 +108,7 @@ eal_thread_loop(__rte_unused void *arg) /* read on our pipe to get commands */ while (1) { + lcore_function_t *f; void *fct_arg; /* wait command */ @@ -110,7 +119,11 @@ eal_thread_loop(__rte_unused void *arg) if (n <= 0) rte_panic("cannot read on configuration pipe\n"); - lcore_config[lcore_id].state = RUNNING; + /* Set the state to 'RUNNING'. Use release order + * since 'state' variable is used as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, RUNNING, + __ATOMIC_RELEASE); /* send ack */ n = 0; @@ -119,18 +132,29 @@ eal_thread_loop(__rte_unused void *arg) if (n < 0) rte_panic("cannot write on configuration pipe\n"); - if (lcore_config[lcore_id].f == NULL) - rte_panic("NULL function pointer\n"); + /* Load 'f' with acquire order to ensure that + * the memory operations from the main thread + * are accessed only after update to 'f' is visible. + * Wait till the update to 'f' is visible to the worker. + */ + while ((f = __atomic_load_n(&lcore_config[lcore_id].f, + __ATOMIC_ACQUIRE)) == NULL) + rte_pause(); /* call the function and store the return value */ fct_arg = lcore_config[lcore_id].arg; - ret = lcore_config[lcore_id].f(fct_arg); + ret = f(fct_arg); lcore_config[lcore_id].ret = ret; lcore_config[lcore_id].f = NULL; lcore_config[lcore_id].arg = NULL; - rte_wmb(); - lcore_config[lcore_id].state = WAIT; + /* Store the state with release order to ensure that + * the memory operations from the worker thread + * are completed before the state is updated. + * Use 'state' as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, WAIT, + __ATOMIC_RELEASE); } /* never reached */ diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c index 0028123c95..54fa93fa62 100644 --- a/lib/eal/windows/eal_thread.c +++ b/lib/eal/windows/eal_thread.c @@ -29,11 +29,19 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id) int m2w = lcore_config[worker_id].pipe_main2worker[1]; int w2m = lcore_config[worker_id].pipe_worker2main[0]; - if (lcore_config[worker_id].state != WAIT) + /* Check if the worker is in 'WAIT' state. Use acquire order + * since 'state' variable is used as the guard variable. + */ + if (__atomic_load_n(&lcore_config[worker_id].state, + __ATOMIC_ACQUIRE) != WAIT) return -EBUSY; - lcore_config[worker_id].f = f; lcore_config[worker_id].arg = arg; + /* Ensure that all the memory operations are completed + * before the worker thread starts running the function. + * Use worker thread function as the guard variable. + */ + __atomic_store_n(&lcore_config[worker_id].f, f, __ATOMIC_RELEASE); /* send message */ n = 0; @@ -84,6 +92,7 @@ eal_thread_loop(void *arg __rte_unused) /* read on our pipe to get commands */ while (1) { + lcore_function_t *f; void *fct_arg; /* wait command */ @@ -94,7 +103,11 @@ eal_thread_loop(void *arg __rte_unused) if (n <= 0) rte_panic("cannot read on configuration pipe\n"); - lcore_config[lcore_id].state = RUNNING; + /* Set the state to 'RUNNING'. Use release order + * since 'state' variable is used as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, RUNNING, + __ATOMIC_RELEASE); /* send ack */ n = 0; @@ -103,18 +116,29 @@ eal_thread_loop(void *arg __rte_unused) if (n < 0) rte_panic("cannot write on configuration pipe\n"); - if (lcore_config[lcore_id].f == NULL) - rte_panic("NULL function pointer\n"); + /* Load 'f' with acquire order to ensure that + * the memory operations from the main thread + * are accessed only after update to 'f' is visible. + * Wait till the update to 'f' is visible to the worker. + */ + while ((f = __atomic_load_n(&lcore_config[lcore_id].f, + __ATOMIC_ACQUIRE)) == NULL) + rte_pause(); /* call the function and store the return value */ fct_arg = lcore_config[lcore_id].arg; - ret = lcore_config[lcore_id].f(fct_arg); + ret = f(fct_arg); lcore_config[lcore_id].ret = ret; lcore_config[lcore_id].f = NULL; lcore_config[lcore_id].arg = NULL; - rte_wmb(); - lcore_config[lcore_id].state = WAIT; + /* Store the state with release order to ensure that + * the memory operations from the worker thread + * are completed before the state is updated. + * Use 'state' as the guard variable. + */ + __atomic_store_n(&lcore_config[lcore_id].state, WAIT, + __ATOMIC_RELEASE); } } From patchwork Mon Oct 25 04:52:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 102728 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 73679A0C45; Mon, 25 Oct 2021 06:53:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C8D2341143; Mon, 25 Oct 2021 06:53:06 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 6DB6441122 for ; Mon, 25 Oct 2021 06:53:03 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E6AFE1063; Sun, 24 Oct 2021 21:53:02 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.12.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DE0F73F73D; Sun, 24 Oct 2021 21:53:02 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, honnappa.nagarahalli@arm.com, konstantin.ananyev@intel.com, david.marchand@redhat.com, feifei.wang2@arm.com Cc: ruifeng.wang@arm.com, nd@arm.com Date: Sun, 24 Oct 2021 23:52:37 -0500 Message-Id: <20211025045237.19878-5-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211025045237.19878-1-honnappa.nagarahalli@arm.com> References: <20210224212018.17576-1-honnappa.nagarahalli@arm.com> <20211025045237.19878-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 4/4] test/ring: use relaxed barriers for ring stress test 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" wrk_cmd variable is used to signal the worker thread to start or stop the stress test loop. Relaxed barriers are used to achieve the same. Signed-off-by: Honnappa Nagarahalli Reviewed-by: Ola Liljedahl Reviewed-by: Feifei Wang --- app/test/test_ring_stress_impl.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/test/test_ring_stress_impl.h b/app/test/test_ring_stress_impl.h index f9ca63b908..2825a9dce6 100644 --- a/app/test/test_ring_stress_impl.h +++ b/app/test/test_ring_stress_impl.h @@ -22,7 +22,7 @@ enum { WRK_CMD_RUN, }; -static volatile uint32_t wrk_cmd __rte_cache_aligned; +static uint32_t wrk_cmd __rte_cache_aligned = WRK_CMD_STOP; /* test run-time in seconds */ static const uint32_t run_time = 60; @@ -197,10 +197,12 @@ test_worker(void *arg, const char *fname, int32_t prcs) fill_ring_elm(&def_elm, UINT32_MAX); fill_ring_elm(&loc_elm, lc); - while (wrk_cmd != WRK_CMD_RUN) { - rte_smp_rmb(); + /* Acquire ordering is not required as the main is not + * really releasing any data through 'wrk_cmd' to + * the worker. + */ + while (__atomic_load_n(&wrk_cmd, __ATOMIC_RELAXED) != WRK_CMD_RUN) rte_pause(); - } cl = rte_rdtsc_precise(); @@ -242,7 +244,7 @@ test_worker(void *arg, const char *fname, int32_t prcs) lcore_stat_update(&la->stats, 1, num, tm0 + tm1, prcs); - } while (wrk_cmd == WRK_CMD_RUN); + } while (__atomic_load_n(&wrk_cmd, __ATOMIC_RELAXED) == WRK_CMD_RUN); cl = rte_rdtsc_precise() - cl; if (prcs == 0) @@ -356,14 +358,12 @@ test_mt1(int (*test)(void *)) } /* signal worker to start test */ - wrk_cmd = WRK_CMD_RUN; - rte_smp_wmb(); + __atomic_store_n(&wrk_cmd, WRK_CMD_RUN, __ATOMIC_RELEASE); usleep(run_time * US_PER_S); /* signal worker to start test */ - wrk_cmd = WRK_CMD_STOP; - rte_smp_wmb(); + __atomic_store_n(&wrk_cmd, WRK_CMD_STOP, __ATOMIC_RELEASE); /* wait for workers and collect stats. */ mc = rte_lcore_id();