DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev
@ 2021-01-14  7:08 Feifei Wang
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test Feifei Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang

For smp barriers in app/eventdev, remove the unnecessary barriers or
replace them with thread fence.

Feifei Wang (5):
  app/eventdev: fix SMP barrier bugs for perf test
  app/eventdev: remove unnecessary barriers for perf test
  app/eventdev: replace wmb with thread fence for perf test
  app/eventdev: remove unnecessary barriers for pipeline test
  app/eventdev: remove unnecessary barriers for order test

 app/test-eventdev/test_order_common.h    |  2 --
 app/test-eventdev/test_perf_common.c     |  4 ----
 app/test-eventdev/test_perf_common.h     | 14 ++++++++++++--
 app/test-eventdev/test_pipeline_common.c |  1 -
 4 files changed, 12 insertions(+), 9 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test
  2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
@ 2021-01-14  7:08 ` Feifei Wang
  2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers " Feifei Wang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  To: Jerin Jacob, Harry van Haaren
  Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang, stable, Ruifeng Wang

This patch fixes RTE SMP barrier bugs for the perf test of eventdev.

For the "perf_process_last_stage" function, wmb after storing
processed_pkts should be moved before it. This is because the worker
lcore should ensure it has really finished data processing, e.g. event
stored into buffers, before the shared variables "w->processed_pkts"are
stored.

For the "perf_process_last_stage_latency", on the one hand, the wmb
should be moved before storing into "w->processed_pkts". The reason is
the same as above. But on the other hand, for "w->latency", wmb is
unnecessary due to data dependency.

Fixes: 2369f73329f8 ("app/testeventdev: add perf queue worker functions")
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-eventdev/test_perf_common.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/test-eventdev/test_perf_common.h b/app/test-eventdev/test_perf_common.h
index ff9705df8..e7233e5a5 100644
--- a/app/test-eventdev/test_perf_common.h
+++ b/app/test-eventdev/test_perf_common.h
@@ -97,8 +97,13 @@ perf_process_last_stage(struct rte_mempool *const pool,
 		void *bufs[], int const buf_sz, uint8_t count)
 {
 	bufs[count++] = ev->event_ptr;
-	w->processed_pkts++;
+
+	/* wmb here ensures event_prt is stored before
+	 * updating the number of processed packets
+	 * for worker lcores
+	 */
 	rte_smp_wmb();
+	w->processed_pkts++;
 
 	if (unlikely(count == buf_sz)) {
 		count = 0;
@@ -116,6 +121,12 @@ perf_process_last_stage_latency(struct rte_mempool *const pool,
 	struct perf_elt *const m = ev->event_ptr;
 
 	bufs[count++] = ev->event_ptr;
+
+	/* wmb here ensures event_prt is stored before
+	 * updating the number of processed packets
+	 * for worker lcores
+	 */
+	rte_smp_wmb();
 	w->processed_pkts++;
 
 	if (unlikely(count == buf_sz)) {
@@ -127,7 +138,6 @@ perf_process_last_stage_latency(struct rte_mempool *const pool,
 	}
 
 	w->latency += latency;
-	rte_smp_wmb();
 	return count;
 }
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers for perf test
  2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test Feifei Wang
@ 2021-01-14  7:08 ` Feifei Wang
  2021-01-25 17:48   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence " Feifei Wang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang, Ruifeng Wang

For "processed_pkts" and "total_latency" functions, no operations should
keep the order that being executed before loading
"worker[i].processed_pkts". Thus rmb is unnecessary before loading.

For "perf_launch_lcores" function, wmb after that the main lcore
updates the variable "t->done", which represents the end of the test
signal, is unnecessary. Because after the main lcore updates this
siginal variable, it will jump out of the launch function loop, and wait
other lcores stop or return error in the main function(evt_main.c).
During this time, there is no important storing operation and thus no
need for wmb.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-eventdev/test_perf_common.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/test-eventdev/test_perf_common.c b/app/test-eventdev/test_perf_common.c
index 955edb752..34cded373 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -224,7 +224,6 @@ processed_pkts(struct test_perf *t)
 	uint8_t i;
 	uint64_t total = 0;
 
-	rte_smp_rmb();
 	for (i = 0; i < t->nb_workers; i++)
 		total += t->worker[i].processed_pkts;
 
@@ -237,7 +236,6 @@ total_latency(struct test_perf *t)
 	uint8_t i;
 	uint64_t total = 0;
 
-	rte_smp_rmb();
 	for (i = 0; i < t->nb_workers; i++)
 		total += t->worker[i].latency;
 
@@ -327,7 +325,6 @@ perf_launch_lcores(struct evt_test *test, struct evt_options *opt,
 					opt->prod_type ==
 					EVT_PROD_TYPE_EVENT_TIMER_ADPTR) {
 					t->done = true;
-					rte_smp_wmb();
 					break;
 				}
 			}
@@ -341,7 +338,6 @@ perf_launch_lcores(struct evt_test *test, struct evt_options *opt,
 				rte_event_dev_dump(opt->dev_id, stdout);
 				evt_err("No schedules for seconds, deadlock");
 				t->done = true;
-				rte_smp_wmb();
 				break;
 			}
 			dead_lock_remaining = remaining;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence for perf test
  2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test Feifei Wang
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers " Feifei Wang
@ 2021-01-14  7:08 ` Feifei Wang
  2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test Feifei Wang
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test Feifei Wang
  4 siblings, 1 reply; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang, Phil Yang, Ruifeng Wang

Simply replace rte_smp barrier with atomic threand fence.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-eventdev/test_perf_common.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/test-eventdev/test_perf_common.h b/app/test-eventdev/test_perf_common.h
index e7233e5a5..9785dc3e2 100644
--- a/app/test-eventdev/test_perf_common.h
+++ b/app/test-eventdev/test_perf_common.h
@@ -98,11 +98,11 @@ perf_process_last_stage(struct rte_mempool *const pool,
 {
 	bufs[count++] = ev->event_ptr;
 
-	/* wmb here ensures event_prt is stored before
-	 * updating the number of processed packets
-	 * for worker lcores
+	/* release fence here ensures event_prt is
+	 * stored before updating the number of
+	 * processed packets for worker lcores
 	 */
-	rte_smp_wmb();
+	rte_atomic_thread_fence(__ATOMIC_RELEASE);
 	w->processed_pkts++;
 
 	if (unlikely(count == buf_sz)) {
@@ -122,11 +122,11 @@ perf_process_last_stage_latency(struct rte_mempool *const pool,
 
 	bufs[count++] = ev->event_ptr;
 
-	/* wmb here ensures event_prt is stored before
-	 * updating the number of processed packets
-	 * for worker lcores
+	/* release fence here ensures event_prt is
+	 * stored before updating the number of
+	 * processed packets for worker lcores
 	 */
-	rte_smp_wmb();
+	rte_atomic_thread_fence(__ATOMIC_RELEASE);
 	w->processed_pkts++;
 
 	if (unlikely(count == buf_sz)) {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test
  2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
                   ` (2 preceding siblings ...)
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence " Feifei Wang
@ 2021-01-14  7:08 ` Feifei Wang
  2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test Feifei Wang
  4 siblings, 1 reply; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang, Ruifeng Wang

For "processed_pkts" function, no operations should keep the order that
being executed before loading "worker[i].processed_pkts".

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-eventdev/test_pipeline_common.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c
index c67be48e9..b47d76743 100644
--- a/app/test-eventdev/test_pipeline_common.c
+++ b/app/test-eventdev/test_pipeline_common.c
@@ -44,7 +44,6 @@ processed_pkts(struct test_pipeline *t)
 	uint8_t i;
 	uint64_t total = 0;
 
-	rte_smp_rmb();
 	for (i = 0; i < t->nb_workers; i++)
 		total += t->worker[i].processed_pkts;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test
  2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
                   ` (3 preceding siblings ...)
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test Feifei Wang
@ 2021-01-14  7:08 ` Feifei Wang
  2021-01-25 17:49   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
  4 siblings, 1 reply; 12+ messages in thread
From: Feifei Wang @ 2021-01-14  7:08 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: dev, Honnappa.Nagarahalli, nd, Feifei Wang, Ruifeng Wang

For the wmb in order_process_stage_1 and order_process_stage_invalid in
the order test, they can be removed. This is because when the test results
are wrong, the worker core writes 'true' to t->err. Then other worker
cores, producer cores and the main core will load the 'error' index and
stop testing. So, for the worker cores, no other storing operation needs
to be guaranteed after this when errors happen.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-eventdev/test_order_common.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/test-eventdev/test_order_common.h b/app/test-eventdev/test_order_common.h
index 5ef840493..cd9d6009e 100644
--- a/app/test-eventdev/test_order_common.h
+++ b/app/test-eventdev/test_order_common.h
@@ -104,7 +104,6 @@ order_process_stage_1(struct test_order *const t,
 			flow, *order_mbuf_seqn(t, ev->mbuf),
 			expected_flow_seq[flow]);
 		t->err = true;
-		rte_smp_wmb();
 	}
 	/*
 	 * Events from an atomic flow of an event queue can be scheduled only to
@@ -123,7 +122,6 @@ order_process_stage_invalid(struct test_order *const t,
 {
 	evt_err("invalid queue %d", ev->queue_id);
 	t->err = true;
-	rte_smp_wmb();
 }
 
 #define ORDER_WORKER_INIT\
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers for perf test
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers " Feifei Wang
@ 2021-01-25 17:48   ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 12+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2021-01-25 17:48 UTC (permalink / raw)
  To: Feifei Wang, Jerin Jacob Kollanukkaran
  Cc: dev, Honnappa.Nagarahalli, nd, Ruifeng Wang

>For "processed_pkts" and "total_latency" functions, no operations
>should
>keep the order that being executed before loading
>"worker[i].processed_pkts". Thus rmb is unnecessary before loading.
>
>For "perf_launch_lcores" function, wmb after that the main lcore
>updates the variable "t->done", which represents the end of the test
>signal, is unnecessary. Because after the main lcore updates this
>siginal variable, it will jump out of the launch function loop, and wait
>other lcores stop or return error in the main function(evt_main.c).
>During this time, there is no important storing operation and thus no
>need for wmb.
>
>Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

>---
> app/test-eventdev/test_perf_common.c | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/app/test-eventdev/test_perf_common.c b/app/test-
>eventdev/test_perf_common.c
>index 955edb752..34cded373 100644
>--- a/app/test-eventdev/test_perf_common.c
>+++ b/app/test-eventdev/test_perf_common.c
>@@ -224,7 +224,6 @@ processed_pkts(struct test_perf *t)
> 	uint8_t i;
> 	uint64_t total = 0;
>
>-	rte_smp_rmb();
> 	for (i = 0; i < t->nb_workers; i++)
> 		total += t->worker[i].processed_pkts;
>
>@@ -237,7 +236,6 @@ total_latency(struct test_perf *t)
> 	uint8_t i;
> 	uint64_t total = 0;
>
>-	rte_smp_rmb();
> 	for (i = 0; i < t->nb_workers; i++)
> 		total += t->worker[i].latency;
>
>@@ -327,7 +325,6 @@ perf_launch_lcores(struct evt_test *test, struct
>evt_options *opt,
> 					opt->prod_type ==
>
>	EVT_PROD_TYPE_EVENT_TIMER_ADPTR) {
> 					t->done = true;
>-					rte_smp_wmb();
> 					break;
> 				}
> 			}
>@@ -341,7 +338,6 @@ perf_launch_lcores(struct evt_test *test, struct
>evt_options *opt,
> 				rte_event_dev_dump(opt->dev_id,
>stdout);
> 				evt_err("No schedules for seconds,
>deadlock");
> 				t->done = true;
>-				rte_smp_wmb();
> 				break;
> 			}
> 			dead_lock_remaining = remaining;
>--
>2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test Feifei Wang
@ 2021-01-25 17:49   ` Pavan Nikhilesh Bhagavatula
  2021-01-26 15:56     ` Jerin Jacob
  0 siblings, 1 reply; 12+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2021-01-25 17:49 UTC (permalink / raw)
  To: Feifei Wang, Jerin Jacob Kollanukkaran
  Cc: dev, Honnappa.Nagarahalli, nd, Ruifeng Wang

>For the wmb in order_process_stage_1 and
>order_process_stage_invalid in
>the order test, they can be removed. This is because when the test
>results
>are wrong, the worker core writes 'true' to t->err. Then other worker
>cores, producer cores and the main core will load the 'error' index and
>stop testing. So, for the worker cores, no other storing operation needs
>to be guaranteed after this when errors happen.
>
>Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>---
> app/test-eventdev/test_order_common.h | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/app/test-eventdev/test_order_common.h b/app/test-
>eventdev/test_order_common.h
>index 5ef840493..cd9d6009e 100644
>--- a/app/test-eventdev/test_order_common.h
>+++ b/app/test-eventdev/test_order_common.h
>@@ -104,7 +104,6 @@ order_process_stage_1(struct test_order
>*const t,
> 			flow, *order_mbuf_seqn(t, ev->mbuf),
> 			expected_flow_seq[flow]);
> 		t->err = true;
>-		rte_smp_wmb();
> 	}
> 	/*
> 	 * Events from an atomic flow of an event queue can be
>scheduled only to
>@@ -123,7 +122,6 @@ order_process_stage_invalid(struct test_order
>*const t,
> {
> 	evt_err("invalid queue %d", ev->queue_id);
> 	t->err = true;
>-	rte_smp_wmb();
> }
>
> #define ORDER_WORKER_INIT\
>--
>2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence for perf test
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence " Feifei Wang
@ 2021-01-25 17:50   ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 12+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2021-01-25 17:50 UTC (permalink / raw)
  To: Feifei Wang, Jerin Jacob Kollanukkaran
  Cc: dev, Honnappa.Nagarahalli, nd, Phil Yang, Ruifeng Wang

>Simply replace rte_smp barrier with atomic threand fence.
>
>Signed-off-by: Phil Yang <phil.yang@arm.com>
>Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>---
> app/test-eventdev/test_perf_common.h | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/app/test-eventdev/test_perf_common.h b/app/test-
>eventdev/test_perf_common.h
>index e7233e5a5..9785dc3e2 100644
>--- a/app/test-eventdev/test_perf_common.h
>+++ b/app/test-eventdev/test_perf_common.h
>@@ -98,11 +98,11 @@ perf_process_last_stage(struct rte_mempool
>*const pool,
> {
> 	bufs[count++] = ev->event_ptr;
>
>-	/* wmb here ensures event_prt is stored before
>-	 * updating the number of processed packets
>-	 * for worker lcores
>+	/* release fence here ensures event_prt is
>+	 * stored before updating the number of
>+	 * processed packets for worker lcores
> 	 */
>-	rte_smp_wmb();
>+	rte_atomic_thread_fence(__ATOMIC_RELEASE);
> 	w->processed_pkts++;
>
> 	if (unlikely(count == buf_sz)) {
>@@ -122,11 +122,11 @@ perf_process_last_stage_latency(struct
>rte_mempool *const pool,
>
> 	bufs[count++] = ev->event_ptr;
>
>-	/* wmb here ensures event_prt is stored before
>-	 * updating the number of processed packets
>-	 * for worker lcores
>+	/* release fence here ensures event_prt is
>+	 * stored before updating the number of
>+	 * processed packets for worker lcores
> 	 */
>-	rte_smp_wmb();
>+	rte_atomic_thread_fence(__ATOMIC_RELEASE);
> 	w->processed_pkts++;
>
> 	if (unlikely(count == buf_sz)) {
>--
>2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test Feifei Wang
@ 2021-01-25 17:50   ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 12+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2021-01-25 17:50 UTC (permalink / raw)
  To: Feifei Wang, Jerin Jacob Kollanukkaran
  Cc: dev, Honnappa.Nagarahalli, nd, Ruifeng Wang

>For "processed_pkts" function, no operations should keep the order
>that
>being executed before loading "worker[i].processed_pkts".
>
>Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>---
> app/test-eventdev/test_pipeline_common.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-
>eventdev/test_pipeline_common.c
>index c67be48e9..b47d76743 100644
>--- a/app/test-eventdev/test_pipeline_common.c
>+++ b/app/test-eventdev/test_pipeline_common.c
>@@ -44,7 +44,6 @@ processed_pkts(struct test_pipeline *t)
> 	uint8_t i;
> 	uint64_t total = 0;
>
>-	rte_smp_rmb();
> 	for (i = 0; i < t->nb_workers; i++)
> 		total += t->worker[i].processed_pkts;
>
>--
>2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test
  2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test Feifei Wang
@ 2021-01-25 17:50   ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 12+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2021-01-25 17:50 UTC (permalink / raw)
  To: Feifei Wang, Jerin Jacob Kollanukkaran, Harry van Haaren
  Cc: dev, Honnappa.Nagarahalli, nd, stable, Ruifeng Wang

>This patch fixes RTE SMP barrier bugs for the perf test of eventdev.
>
>For the "perf_process_last_stage" function, wmb after storing
>processed_pkts should be moved before it. This is because the worker
>lcore should ensure it has really finished data processing, e.g. event
>stored into buffers, before the shared variables "w-
>>processed_pkts"are
>stored.
>
>For the "perf_process_last_stage_latency", on the one hand, the wmb
>should be moved before storing into "w->processed_pkts". The reason
>is
>the same as above. But on the other hand, for "w->latency", wmb is
>unnecessary due to data dependency.
>
>Fixes: 2369f73329f8 ("app/testeventdev: add perf queue worker
>functions")
>Cc: jerinj@marvell.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
>Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

>---
> app/test-eventdev/test_perf_common.h | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/app/test-eventdev/test_perf_common.h b/app/test-
>eventdev/test_perf_common.h
>index ff9705df8..e7233e5a5 100644
>--- a/app/test-eventdev/test_perf_common.h
>+++ b/app/test-eventdev/test_perf_common.h
>@@ -97,8 +97,13 @@ perf_process_last_stage(struct rte_mempool
>*const pool,
> 		void *bufs[], int const buf_sz, uint8_t count)
> {
> 	bufs[count++] = ev->event_ptr;
>-	w->processed_pkts++;
>+
>+	/* wmb here ensures event_prt is stored before
>+	 * updating the number of processed packets
>+	 * for worker lcores
>+	 */
> 	rte_smp_wmb();
>+	w->processed_pkts++;
>
> 	if (unlikely(count == buf_sz)) {
> 		count = 0;
>@@ -116,6 +121,12 @@ perf_process_last_stage_latency(struct
>rte_mempool *const pool,
> 	struct perf_elt *const m = ev->event_ptr;
>
> 	bufs[count++] = ev->event_ptr;
>+
>+	/* wmb here ensures event_prt is stored before
>+	 * updating the number of processed packets
>+	 * for worker lcores
>+	 */
>+	rte_smp_wmb();
> 	w->processed_pkts++;
>
> 	if (unlikely(count == buf_sz)) {
>@@ -127,7 +138,6 @@ perf_process_last_stage_latency(struct
>rte_mempool *const pool,
> 	}
>
> 	w->latency += latency;
>-	rte_smp_wmb();
> 	return count;
> }
>
>--
>2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test
  2021-01-25 17:49   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
@ 2021-01-26 15:56     ` Jerin Jacob
  0 siblings, 0 replies; 12+ messages in thread
From: Jerin Jacob @ 2021-01-26 15:56 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula
  Cc: Feifei Wang, Jerin Jacob Kollanukkaran, dev,
	Honnappa.Nagarahalli, nd, Ruifeng Wang

On Mon, Jan 25, 2021 at 11:19 PM Pavan Nikhilesh Bhagavatula
<pbhagavatula@marvell.com> wrote:
>
> >For the wmb in order_process_stage_1 and
> >order_process_stage_invalid in
> >the order test, they can be removed. This is because when the test
> >results
> >are wrong, the worker core writes 'true' to t->err. Then other worker
> >cores, producer cores and the main core will load the 'error' index and
> >stop testing. So, for the worker cores, no other storing operation needs
> >to be guaranteed after this when errors happen.
> >
> >Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> >Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>


Series applied to dpdk-next-eventdev/for-main. Thanks.


> >---
> > app/test-eventdev/test_order_common.h | 2 --
> > 1 file changed, 2 deletions(-)
> >
> >diff --git a/app/test-eventdev/test_order_common.h b/app/test-
> >eventdev/test_order_common.h
> >index 5ef840493..cd9d6009e 100644
> >--- a/app/test-eventdev/test_order_common.h
> >+++ b/app/test-eventdev/test_order_common.h
> >@@ -104,7 +104,6 @@ order_process_stage_1(struct test_order
> >*const t,
> >                       flow, *order_mbuf_seqn(t, ev->mbuf),
> >                       expected_flow_seq[flow]);
> >               t->err = true;
> >-              rte_smp_wmb();
> >       }
> >       /*
> >        * Events from an atomic flow of an event queue can be
> >scheduled only to
> >@@ -123,7 +122,6 @@ order_process_stage_invalid(struct test_order
> >*const t,
> > {
> >       evt_err("invalid queue %d", ev->queue_id);
> >       t->err = true;
> >-      rte_smp_wmb();
> > }
> >
> > #define ORDER_WORKER_INIT\
> >--
> >2.25.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-26 15:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14  7:08 [dpdk-dev] [PATCH v1 0/5] refactor smp barriers in app/eventdev Feifei Wang
2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 1/5] app/eventdev: fix SMP barrier bugs for perf test Feifei Wang
2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 2/5] app/eventdev: remove unnecessary barriers " Feifei Wang
2021-01-25 17:48   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 3/5] app/eventdev: replace wmb with thread fence " Feifei Wang
2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 4/5] app/eventdev: remove unnecessary barriers for pipeline test Feifei Wang
2021-01-25 17:50   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-01-14  7:08 ` [dpdk-dev] [PATCH v1 5/5] app/eventdev: remove unnecessary barriers for order test Feifei Wang
2021-01-25 17:49   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2021-01-26 15:56     ` Jerin Jacob

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git