From: Harry van Haaren <harry.van.haaren@intel.com>
To: dev@dpdk.org
Cc: Harry van Haaren <harry.van.haaren@intel.com>
Subject: [dpdk-dev] [PATCH v2 08/15] service: fix and refactor atomic service accesses
Date: Mon, 21 Aug 2017 13:58:09 +0100 [thread overview]
Message-ID: <1503320296-51122-9-git-send-email-harry.van.haaren@intel.com> (raw)
In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com>
This commit fixes an issue in the service runner function,
where the atomic value was not cleared on exiting the service
function. This resulted in future attempts to run the service
to appear like the function was running, however it was in
reality deadlocked.
This commit refactors the atomic handling to be more readable,
by splitting the implementation code into a new static inline
function. The remaining flow control of atomics in the existing
function is refactored for readability.
Fixes: 21698354c832 ("service: introduce service cores concept")
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
doc/guides/rel_notes/release_17_11.rst | 7 ++++++
lib/librte_eal/common/rte_service.c | 45 ++++++++++++++++++++--------------
2 files changed, 33 insertions(+), 19 deletions(-)
diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst
index 170f4f9..6e6ba1c 100644
--- a/doc/guides/rel_notes/release_17_11.rst
+++ b/doc/guides/rel_notes/release_17_11.rst
@@ -65,6 +65,13 @@ Resolved Issues
EAL
~~~
+* **Service core fails to call service callback due to atomic lock**
+
+ In a specific configuration of multi-thread unsafe services and service
+ cores, a service core previously did not correctly release the atomic lock
+ on the service. This would result in the cores polling the service, but it
+ looked like another thread was executing the service callback. The logic for
+ atomic locking of the services has been fixed and refactored for readability.
Drivers
~~~~~~~
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 1bfd149..6291c0c 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -296,6 +296,23 @@ rte_service_runstate_get(uint32_t id)
return (s->runstate == RUNSTATE_RUNNING) && (s->num_mapped_cores > 0);
}
+static inline void
+rte_service_runner_do_callback(struct rte_service_spec_impl *s,
+ struct core_state *cs, uint32_t service_idx)
+{
+ void *userdata = s->spec.callback_userdata;
+
+ if (service_stats_enabled(s)) {
+ uint64_t start = rte_rdtsc();
+ s->spec.callback(userdata);
+ uint64_t end = rte_rdtsc();
+ s->cycles_spent += end - start;
+ cs->calls_per_service[service_idx]++;
+ s->calls++;
+ } else
+ s->spec.callback(userdata);
+}
+
static int32_t
rte_service_runner_func(void *arg)
{
@@ -315,26 +332,16 @@ rte_service_runner_func(void *arg)
/* check do we need cmpset, if MT safe or <= 1 core
* mapped, atomic ops are not required.
*/
- const int need_cmpset = !((service_mt_safe(s) == 0) &&
- (s->num_mapped_cores > 1));
- uint32_t *lock = (uint32_t *)&s->execute_lock;
-
- if (need_cmpset || rte_atomic32_cmpset(lock, 0, 1)) {
- void *userdata = s->spec.callback_userdata;
-
- if (service_stats_enabled(s)) {
- uint64_t start = rte_rdtsc();
- s->spec.callback(userdata);
- uint64_t end = rte_rdtsc();
- s->cycles_spent += end - start;
- cs->calls_per_service[i]++;
- s->calls++;
- } else
- s->spec.callback(userdata);
-
- if (need_cmpset)
+ const int use_atomics = (service_mt_safe(s) == 0) &&
+ (s->num_mapped_cores > 1);
+ if (use_atomics) {
+ uint32_t *lock = (uint32_t *)&s->execute_lock;
+ if (rte_atomic32_cmpset(lock, 0, 1)) {
+ rte_service_runner_do_callback(s, cs, i);
rte_atomic32_clear(&s->execute_lock);
- }
+ }
+ } else
+ rte_service_runner_do_callback(s, cs, i);
}
rte_smp_rmb();
--
2.7.4
next prev parent reply other threads:[~2017-08-21 12:58 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-15 12:32 [dpdk-dev] [PATCH 0/8] service: rework for usability Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 1/8] service: rework probe and get name to use ids Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 2/8] service: rework lcore to service map functions Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 3/8] service: rework register to return service id Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 4/8] service: rework service start stop to runstate Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 5/8] service: rework service stats functions Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 6/8] service: rework unregister api to use integers Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 7/8] service: rework get by name function to use id Harry van Haaren
2017-08-15 12:32 ` [dpdk-dev] [PATCH 8/8] service: clarify documentation for register Harry van Haaren
2017-08-16 11:16 ` [dpdk-dev] [PATCH 0/8] service: rework for usability Neil Horman
2017-08-16 11:31 ` Van Haaren, Harry
2017-08-16 12:07 ` Van Haaren, Harry
2017-08-16 13:27 ` Neil Horman
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 00/15] service: API improvements and updates Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 01/15] service: rework probe and get name to use ids Harry van Haaren
2017-08-30 19:25 ` Pavan Nikhilesh Bhagavatula
2017-09-04 14:32 ` Pavan Nikhilesh Bhagavatula
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 02/15] service: rework lcore to service map functions Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 03/15] service: rework register to return service id Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 04/15] service: rework service start stop to runstate Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 05/15] service: rework service stats functions Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 06/15] service: rework unregister api to use integers Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 07/15] service: rework get by name function to use id Harry van Haaren
2017-08-21 12:58 ` Harry van Haaren [this message]
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 09/15] service: fix loops to always scan all services Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 10/15] service: fix return values of functions to 0 or 1 Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 11/15] service: fix lcore in wait state in lcore add Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 12/15] service: reset core call stats on dump Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 13/15] service: add component runstate Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 14/15] service: clarify documentation for register Harry van Haaren
2017-08-21 12:58 ` [dpdk-dev] [PATCH v2 15/15] docs: add notes on service cores API updates Harry van Haaren
2017-09-15 11:51 ` [dpdk-dev] [PATCH v2 00/15] service: API improvements and updates Thomas Monjalon
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=1503320296-51122-9-git-send-email-harry.van.haaren@intel.com \
--to=harry.van.haaren@intel.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).