[dpdk-stable] patch 'service: avoid false sharing on core state' has been queued to LTS release 18.11.7

[Kevin Traynor]

Hi,

FYI, your patch has been queued to LTS release 18.11.7

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 02/13/20. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/kevintraynor/dpdk-stable-queue

This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable-queue/commit/e07f5531f86e3dfadda1f52129cc2b61370e3e74

Thanks.

Kevin.

---
>From e07f5531f86e3dfadda1f52129cc2b61370e3e74 Mon Sep 17 00:00:00 2001
From: Nikhil Rao <nikhil.rao at intel.com>
Date: Mon, 16 Sep 2019 15:31:02 +0530
Subject: [PATCH] service: avoid false sharing on core state

[ upstream commit e484ccddbe1b41886fef1e445ef2fdfa55086198 ]

For a valid service, the core mask of the service
is checked against the current core and the corresponding
entry in the active_on_lcore array is set or reset.

Upto 8 cores share the same cache line for their
service active_on_lcore array entries since each entry is a uint8_t.
Some number of these entries also share the cache line with
the internal_flags member of struct rte_service_spec_impl,
hence this false sharing also makes the service_valid() check
expensive.

Eliminate false sharing by moving the active_on_lcore array to
a per-core data structure. The array is now indexed by service id.

Signed-off-by: Nikhil Rao <nikhil.rao at intel.com>
Acked-by: Gage Eads <gage.eads at intel.com>
Acked-by: Harry van Haaren <harry.van.haaren at intel.com>
---
 lib/librte_eal/common/rte_service.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index e7d3e51443..97449460dc 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -53,5 +53,4 @@ struct rte_service_spec_impl {
 	uint64_t calls;
 	uint64_t cycles_spent;
-	uint8_t active_on_lcore[RTE_MAX_LCORE];
 } __rte_cache_aligned;
 
@@ -62,5 +61,5 @@ struct core_state {
 	uint8_t runstate; /* running or stopped */
 	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;
 	uint64_t calls_per_service[RTE_SERVICE_NUM_MAX];
@@ -346,5 +345,5 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
 
 static inline int32_t
-service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask)
+service_run(uint32_t i, struct core_state *cs, uint64_t service_mask)
 {
 	if (!service_valid(i))
@@ -354,9 +353,9 @@ service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask)
 			s->app_runstate != RUNSTATE_RUNNING ||
 			!(service_mask & (UINT64_C(1) << i))) {
-		s->active_on_lcore[lcore] = 0;
+		cs->service_active_on_lcore[i] = 0;
 		return -ENOEXEC;
 	}
 
-	s->active_on_lcore[lcore] = 1;
+	cs->service_active_on_lcore[i] = 1;
 
 	/* check do we need cmpset, if MT safe or <= 1 core
@@ -381,5 +380,4 @@ rte_service_may_be_active(uint32_t id)
 {
 	uint32_t ids[RTE_MAX_LCORE] = {0};
-	struct rte_service_spec_impl *s = &rte_services[id];
 	int32_t lcore_count = rte_service_lcore_list(ids, RTE_MAX_LCORE);
 	int i;
@@ -389,5 +387,5 @@ rte_service_may_be_active(uint32_t id)
 
 	for (i = 0; i < lcore_count; i++) {
-		if (s->active_on_lcore[ids[i]])
+		if (lcore_states[i].service_active_on_lcore[id])
 			return 1;
 	}
@@ -420,5 +418,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
 	}
 
-	int ret = service_run(id, rte_lcore_id(), cs, UINT64_MAX);
+	int ret = service_run(id, cs, UINT64_MAX);
 
 	if (serialize_mt_unsafe)
@@ -441,5 +439,5 @@ rte_service_runner_func(void *arg)
 		for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
 			/* return value ignored as no change to code flow */
-			service_run(i, lcore, cs, service_mask);
+			service_run(i, cs, service_mask);
 		}
 
-- 
2.21.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-02-07 15:08:19.343140553 +0000
+++ 0035-service-avoid-false-sharing-on-core-state.patch	2020-02-07 15:08:17.571062192 +0000
@@ -1 +1 @@
-From e484ccddbe1b41886fef1e445ef2fdfa55086198 Mon Sep 17 00:00:00 2001
+From e07f5531f86e3dfadda1f52129cc2b61370e3e74 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit e484ccddbe1b41886fef1e445ef2fdfa55086198 ]
+
@@ -28 +30 @@
-index fe09077200..beb9691b23 100644
+index e7d3e51443..97449460dc 100644
@@ -31 +33 @@
-@@ -52,5 +52,4 @@ struct rte_service_spec_impl {
+@@ -53,5 +53,4 @@ struct rte_service_spec_impl {
@@ -37 +39 @@
-@@ -61,5 +60,5 @@ struct core_state {
+@@ -62,5 +61,5 @@ struct core_state {
@@ -44 +46 @@
-@@ -345,5 +344,5 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
+@@ -346,5 +345,5 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s,
@@ -51 +53 @@
-@@ -353,9 +352,9 @@ service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask)
+@@ -354,9 +353,9 @@ service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask)
@@ -63 +65 @@
-@@ -380,5 +379,4 @@ rte_service_may_be_active(uint32_t id)
+@@ -381,5 +380,4 @@ rte_service_may_be_active(uint32_t id)
@@ -69 +71 @@
-@@ -388,5 +386,5 @@ rte_service_may_be_active(uint32_t id)
+@@ -389,5 +387,5 @@ rte_service_may_be_active(uint32_t id)
@@ -76 +78 @@
-@@ -419,5 +417,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
+@@ -420,5 +418,5 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe)
@@ -83 +85 @@
-@@ -440,5 +438,5 @@ rte_service_runner_func(void *arg)
+@@ -441,5 +439,5 @@ rte_service_runner_func(void *arg)