From: Feifei Wang <Feifei.Wang2@arm.com>
To: "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
"jerinj@marvell.com" <jerinj@marvell.com>,
Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
"harry.van.haaren@intel.com" <harry.van.haaren@intel.com>,
"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
"dmitry.kozliuk@gmail.com" <dmitry.kozliuk@gmail.com>,
"navasile@linux.microsoft.com" <navasile@linux.microsoft.com>,
"dmitrym@microsoft.com" <dmitrym@microsoft.com>,
"pallavi.kadam@intel.com" <pallavi.kadam@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Ruifeng Wang <Ruifeng.Wang@arm.com>, nd <nd@arm.com>
Subject: [dpdk-dev] 回复: [RFC 3/5] eal: lcore state FINISHED is not required
Date: Thu, 25 Feb 2021 08:44:14 +0000 [thread overview]
Message-ID: <DBBPR08MB44118CEC7065CC2314E8869DC89E9@DBBPR08MB4411.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <AM5PR0802MB2465B62994239817E0AC46C59E9E9@AM5PR0802MB2465.eurprd08.prod.outlook.com>
Hi, Honnappa
> -----Original Message-----
> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Sent: Thursday, February 25, 2021 5:20 AM
> To: hemant.agrawal@nxp.com; Nipun.gupta@nxp.com; jerinj@marvell.com;
> Harry van Haaren <harry.van.haaren@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>;
> Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Dmitry Malloy
> <dmitrym@microsoft.com>; Pallavi Kadam <pallavi.kadam@intel.com>
> Cc: dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Feifei Wang (Arm Technology China)
> <Feifei.Wang@arm.com>; nd <nd@arm.com>
> Subject: [RFC 3/5] eal: lcore state FINISHED is not required
>
> 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.
I am not sure "FINISHED" is necessary to be removed, and I propose some of my profiles for discussion.
There are three states for lcore now:
"WAIT": indicate lcore can start working
"RUNNING": indicate lcore is working
"FINISHED": indicate lcore has finished its working and wait to be reset
From the description above, we can find "FINISHED" is different from "WAIT", it can shows that lcore
has done the work and finished it. Thus, if we remove "FINISHED", maybe we will not know whether
the lcore finishes its work or just doesn't start, because this two state has the same tag "WAIT".
Furthermore, consider such a scenario:
Core 1 need to monitor Core 2 state, if Core 2 finishes one task, Core 1 can start its working.
However, if there is only one tag "WAIT", Core 1 maybe start its work at the wrong time, when
Core 2 still does not start its task at state "WAIT".
This is just my guess, and at present, there is no similar application scenario in dpdk.
On the other hand, if we decide to remove "FINISHED", please consider the following files:
1. lib/librte_eal/linux/eal_thread.c: line 31
lib/librte_eal/windows/eal_thread.c: line 22
lib/librte_eal/freebsd/eal_thread.c: line 31
2. lib/librte_eal/include/rte_launch.h: line 24, 44, 121, 123, 131
3. examples/l2fwd-keepalive/main.c: line 510
rte_eal_wait_lcore(id_core) can be removed. Because the core state has been checked as "WAIT",
this is a redundant operation
Best Regards
Feifei
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> ---
> 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 | 2 +-
> lib/librte_eal/common/eal_common_launch.c | 7 ++-----
> lib/librte_eal/freebsd/eal_thread.c | 2 +-
> lib/librte_eal/linux/eal_thread.c | 8 +-------
> lib/librte_eal/windows/eal_thread.c | 8 +-------
> 8 files changed, 10 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/event/dpaa2/dpaa2_eventdev_selftest.c
> b/drivers/event/dpaa2/dpaa2_eventdev_selftest.c
> index cd7311a94..bbbd20951 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 528f99dd8..d7b0d2211 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 e4bfb3a0f..7847a8645 100644
> --- a/drivers/event/sw/sw_evdev_selftest.c
> +++ b/drivers/event/sw/sw_evdev_selftest.c
> @@ -3138,8 +3138,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 e4c2b2793..dd777c46a 100644
> --- a/examples/l2fwd-keepalive/main.c
> +++ b/examples/l2fwd-keepalive/main.c
> @@ -506,7 +506,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) {
> +if (rte_eal_get_lcore_state(id_core) == WAIT) {
> rte_eal_wait_lcore(id_core);
> rte_eal_remote_launch(l2fwd_launch_one_lcore, NULL, id_core); } else
> { diff --git a/lib/librte_eal/common/eal_common_launch.c
> b/lib/librte_eal/common/eal_common_launch.c
> index 34f854ad8..78fd94026 100644
> --- a/lib/librte_eal/common/eal_common_launch.c
> +++ b/lib/librte_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/librte_eal/freebsd/eal_thread.c
> b/lib/librte_eal/freebsd/eal_thread.c
> index 17b8f3996..6d6f1e2fd 100644
> --- a/lib/librte_eal/freebsd/eal_thread.c
> +++ b/lib/librte_eal/freebsd/eal_thread.c
> @@ -140,7 +140,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/librte_eal/linux/eal_thread.c
> b/lib/librte_eal/linux/eal_thread.c
> index a0a009104..7b9463df3 100644
> --- a/lib/librte_eal/linux/eal_thread.c
> +++ b/lib/librte_eal/linux/eal_thread.c
> @@ -141,13 +141,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/librte_eal/windows/eal_thread.c
> b/lib/librte_eal/windows/eal_thread.c
> index 7a9277c51..35d059a30 100644
> --- a/lib/librte_eal/windows/eal_thread.c
> +++ b/lib/librte_eal/windows/eal_thread.c
> @@ -125,13 +125,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;
> }
> }
>
> --
> 2.17.1
>
next prev parent reply other threads:[~2021-02-25 8:44 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-24 21:20 [dpdk-dev] [RFC 0/5] Use correct memory ordering in eal functions Honnappa Nagarahalli
2021-02-24 21:20 ` [dpdk-dev] [RFC 1/5] eal: reset lcore function pointer and argument Honnappa Nagarahalli
2021-02-24 21:20 ` [dpdk-dev] [RFC 2/5] eal: ensure memory operations are visible to worker Honnappa Nagarahalli
2021-02-24 21:20 ` [dpdk-dev] [RFC 3/5] eal: lcore state FINISHED is not required Honnappa Nagarahalli
[not found] ` <AM5PR0802MB2465B62994239817E0AC46C59E9E9@AM5PR0802MB2465.eurprd08.prod.outlook.com>
2021-02-25 8:44 ` Feifei Wang [this message]
2021-02-25 23:33 ` Honnappa Nagarahalli
2021-02-26 8:26 ` Thomas Monjalon
2021-03-02 3:13 ` Honnappa Nagarahalli
2021-03-19 13:42 ` Ananyev, Konstantin
2021-03-30 2:54 ` Honnappa Nagarahalli
2021-03-01 5:55 ` [dpdk-dev] 回复: " Feifei Wang
2021-02-24 21:20 ` [dpdk-dev] [RFC 4/5] eal: ensure memory operations are visible to main Honnappa Nagarahalli
2021-02-24 21:20 ` [dpdk-dev] [RFC 5/5] test/ring: use relaxed barriers for ring stress test Honnappa Nagarahalli
2021-03-01 16:41 ` [dpdk-dev] [RFC 0/5] Use correct memory ordering in eal functions Stephen Hemminger
2021-03-02 16:06 ` Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 0/6] " Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 1/6] eal: reset lcore function pointer and argument Honnappa Nagarahalli
2021-09-10 7:49 ` Bruce Richardson
2021-09-10 8:12 ` David Marchand
2021-09-11 22:19 ` Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 2/6] eal: ensure memory operations are visible to worker Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 3/6] eal: lcore state FINISHED is not required Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 4/6] eal: update rte_eal_wait_lcore definition Honnappa Nagarahalli
2021-09-09 23:37 ` Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 5/6] eal: ensure memory operations are visible to main Honnappa Nagarahalli
2021-09-09 23:13 ` [dpdk-dev] [PATCH v2 6/6] test/ring: use relaxed barriers for ring stress test Honnappa Nagarahalli
2021-10-07 11:55 ` Ananyev, Konstantin
2021-10-07 23:40 ` Honnappa Nagarahalli
2021-10-25 4:52 ` [dpdk-dev] [PATCH v3 0/4] Use correct memory ordering in eal functions Honnappa Nagarahalli
2021-10-25 4:52 ` [dpdk-dev] [PATCH v3 1/4] eal: reset lcore function pointer and argument Honnappa Nagarahalli
2021-10-25 4:52 ` [dpdk-dev] [PATCH v3 2/4] eal: lcore state FINISHED is not required Honnappa Nagarahalli
2021-10-25 4:52 ` [dpdk-dev] [PATCH v3 3/4] eal: use correct memory ordering Honnappa Nagarahalli
2021-10-25 4:52 ` [dpdk-dev] [PATCH v3 4/4] test/ring: use relaxed barriers for ring stress test Honnappa Nagarahalli
2021-10-25 16:23 ` [dpdk-dev] [PATCH v3 0/4] Use correct memory ordering in eal functions David Marchand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DBBPR08MB44118CEC7065CC2314E8869DC89E9@DBBPR08MB4411.eurprd08.prod.outlook.com \
--to=feifei.wang2@arm.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Ruifeng.Wang@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=dmitry.kozliuk@gmail.com \
--cc=dmitrym@microsoft.com \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerinj@marvell.com \
--cc=navasile@linux.microsoft.com \
--cc=nd@arm.com \
--cc=nipun.gupta@nxp.com \
--cc=pallavi.kadam@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).