DPDK patches and discussions
 help / color / mirror / Atom feed
From: Harry van Haaren <harry.van.haaren@intel.com>
To: dev@dpdk.org
Cc: david.marchand@redhat.com, Harry van Haaren <harry.van.haaren@intel.com>
Subject: [dpdk-dev] [PATCH v6 1/2] service: add API to retrieve service core active
Date: Mon, 14 Sep 2020 15:31:17 +0100	[thread overview]
Message-ID: <20200914143118.84791-1-harry.van.haaren@intel.com> (raw)
In-Reply-To: <20200724134506.11959-1-harry.van.haaren@intel.com>

This commit adds a new experimental API which allows the user
to retrieve the active state of an lcore. Knowing when the service
lcore is completed its polling loop can be useful to applications
to avoid race conditions when e.g. finalizing statistics.

The service thread itself now has a variable to indicate if its
thread is active. When zero the service thread has completed its
service, and has returned from the service_runner_func() function.

Suggested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

---

v5:
- Fix typos (robot)

v4:
- Use _may_be_ style API for lcore_active (Honnappa)
- Fix missing tab indent (Honnappa)
- Add 'lcore' to doxygen retval description (Honnappa)

@Honnappa: Please note i did not update the doxygen title of the
lcore_may_be_active() function, as the current description is more
accurate than making it more consistent with other functions.

v3:
- Change service lcore stores to SEQ_CST (Honnappa, David)
- Change control thread load to ACQ (Honnappa, David)
- Comment reasons for SEQ_CST/ACQ (Honnappa, David)
- Add comments to Doxygen for _stop() and _lcore_active() (Honnappa, David)
- Add Phil's review tag from ML
---
 lib/librte_eal/common/rte_service.c  | 21 +++++++++++++++++++++
 lib/librte_eal/include/rte_service.h | 22 +++++++++++++++++++++-
 lib/librte_eal/rte_eal_version.map   |  1 +
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 6a0e0ff65d..98565bbef3 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -65,6 +65,7 @@ struct core_state {
 	/* map of services IDs are run on this core */
 	uint64_t service_mask;
 	uint8_t runstate; /* running or stopped */
+	uint8_t thread_active; /* indicates when thread is in service_run() */
 	uint8_t is_service_core; /* set if core is currently a service core */
 	uint8_t service_active_on_lcore[RTE_SERVICE_NUM_MAX];
 	uint64_t loops;
@@ -457,6 +458,8 @@ service_runner_func(void *arg)
 	const int lcore = rte_lcore_id();
 	struct core_state *cs = &lcore_states[lcore];
 
+	__atomic_store_n(&cs->thread_active, 1, __ATOMIC_SEQ_CST);
+
 	/* runstate act as the guard variable. Use load-acquire
 	 * memory order here to synchronize with store-release
 	 * in runstate update functions.
@@ -475,9 +478,27 @@ service_runner_func(void *arg)
 		cs->loops++;
 	}
 
+	/* Use SEQ CST memory ordering to avoid any re-ordering around
+	 * this store, ensuring that once this store is visible, the service
+	 * lcore thread really is done in service cores code.
+	 */
+	__atomic_store_n(&cs->thread_active, 0, __ATOMIC_SEQ_CST);
 	return 0;
 }
 
+int32_t
+rte_service_lcore_may_be_active(uint32_t lcore)
+{
+	if (lcore >= RTE_MAX_LCORE || !lcore_states[lcore].is_service_core)
+		return -EINVAL;
+
+	/* Load thread_active using ACQUIRE to avoid instructions dependent on
+	 * the result being re-ordered before this load completes.
+	 */
+	return __atomic_load_n(&lcore_states[lcore].thread_active,
+			       __ATOMIC_ACQUIRE);
+}
+
 int32_t
 rte_service_lcore_count(void)
 {
diff --git a/lib/librte_eal/include/rte_service.h b/lib/librte_eal/include/rte_service.h
index e2d0a6dd32..ca9950d091 100644
--- a/lib/librte_eal/include/rte_service.h
+++ b/lib/librte_eal/include/rte_service.h
@@ -249,7 +249,11 @@ int32_t rte_service_lcore_start(uint32_t lcore_id);
  * Stop a service core.
  *
  * Stopping a core makes the core become idle, but remains  assigned as a
- * service core.
+ * service core. Note that the service lcore thread may not have returned from
+ * the service it is running when this API returns.
+ *
+ * The *rte_service_lcore_may_be_active* API can be used to check if the
+ * service lcore is * still active.
  *
  * @retval 0 Success
  * @retval -EINVAL Invalid *lcore_id* provided
@@ -261,6 +265,22 @@ int32_t rte_service_lcore_start(uint32_t lcore_id);
  */
 int32_t rte_service_lcore_stop(uint32_t lcore_id);
 
+/**
+ * Reports if a service lcore is currently running.
+ *
+ * This function returns if the core has finished service cores code, and has
+ * returned to EAL control. If *rte_service_lcore_stop* has been called but
+ * the lcore has not returned to EAL yet, it might be required to wait and call
+ * this function again. The amount of time to wait before the core returns
+ * depends on the duration of the services being run.
+ *
+ * @retval 0 Service thread is not active, and lcore has been returned to EAL.
+ * @retval 1 Service thread is in the service core polling loop.
+ * @retval -EINVAL Invalid *lcore_id* provided.
+ */
+__rte_experimental
+int32_t rte_service_lcore_may_be_active(uint32_t lcore_id);
+
 /**
  * Adds lcore to the list of service cores.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 0b18e2ef85..55f611d071 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -395,6 +395,7 @@ EXPERIMENTAL {
 	rte_lcore_dump;
 	rte_lcore_iterate;
 	rte_mp_disable;
+	rte_service_lcore_may_be_active;
 	rte_thread_register;
 	rte_thread_unregister;
 };
-- 
2.17.1


  parent reply	other threads:[~2020-09-14 14:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200720120850eucas1p10debcafa273d244a7e63d225c50cc9df@eucas1p1.samsung.com>
2020-07-20 12:09 ` [dpdk-dev] [PATCH] service: fix stop API to wait for service thread Harry van Haaren
2020-07-20 12:51   ` Lukasz Wojciechowski
2020-07-20 14:20     ` Van Haaren, Harry
2020-07-20 14:38   ` [dpdk-dev] [PATCH v2 1/2] service: add API to retrieve service core active Harry van Haaren
2020-07-20 14:38     ` [dpdk-dev] [PATCH v2 2/2] test/service: fix race condition on stopping lcore Harry van Haaren
2020-07-20 17:45       ` Lukasz Wojciechowski
2020-07-21  8:38       ` Phil Yang
2020-07-22 10:26         ` Van Haaren, Harry
2020-07-20 17:45     ` [dpdk-dev] [PATCH v2 1/2] service: add API to retrieve service core active Lukasz Wojciechowski
2020-07-21  7:47     ` Phil Yang
2020-07-21 19:43     ` Honnappa Nagarahalli
2020-07-21 19:50       ` David Marchand
2020-07-21 20:23         ` Honnappa Nagarahalli
2020-07-22 10:14           ` Van Haaren, Harry
2020-07-22 18:50             ` Honnappa Nagarahalli
2020-07-23 16:59               ` Van Haaren, Harry
2020-07-22 10:37     ` [dpdk-dev] [PATCH v3 " Harry van Haaren
2020-07-22 10:37       ` [dpdk-dev] [PATCH v3 2/2] test/service: fix race condition on stopping lcore Harry van Haaren
2020-07-22 21:40         ` Honnappa Nagarahalli
2020-07-22 21:39       ` [dpdk-dev] [PATCH v3 1/2] service: add API to retrieve service core active Honnappa Nagarahalli
2020-07-24 12:45       ` [dpdk-dev] [PATCH v4 " Harry van Haaren
2020-07-24 12:45         ` [dpdk-dev] [PATCH v4 2/2] test/service: fix race condition on stopping lcore Harry van Haaren
2020-07-24 13:45         ` [dpdk-dev] [PATCH v5 1/2] service: add API to retrieve service core active Harry van Haaren
2020-07-24 13:45           ` [dpdk-dev] [PATCH v5 2/2] test/service: fix race condition on stopping lcore Harry van Haaren
2020-09-14  8:36             ` David Marchand
2020-09-14 14:33               ` Van Haaren, Harry
2020-09-14 14:31           ` Harry van Haaren [this message]
2020-09-14 14:31             ` [dpdk-dev] [PATCH v6 " Harry van Haaren
2020-09-21 14:51               ` David Marchand
2020-10-13 19:45                 ` David Marchand
2020-10-15  8:11                   ` Van Haaren, Harry

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=20200914143118.84791-1-harry.van.haaren@intel.com \
    --to=harry.van.haaren@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    /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).