diff mbox series

app/eventdev: remove unnecessary barrier for order test

Message ID 20210510061148.1674641-1-feifei.wang2@arm.com (mailing list archive)
State Accepted, archived
Delegated to: Jerin Jacob
Headers
Series app/eventdev: remove unnecessary barrier for order test |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Feifei Wang May 10, 2021, 6:11 a.m. UTC 
  For "order_launch_lcores" function, wmb after that the main lcore
updates the variable "t->err", 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 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>
Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
---
 app/test-eventdev/test_order_common.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Feifei Wang May 18, 2021, 9:27 a.m. UTC | #1 
Hi, Jerin

Sorry to disturb you. Would you please help me review this patch, thanks very much.


Best Regards
Feifei

> -----Original Message-----
> From: Feifei Wang <feifei.wang2@arm.com>
> Sent: Monday, May 10, 2021 2:12 PM
> To: jerinj@marvell.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Subject: [PATCH] app/eventdev: remove unnecessary barrier for order test
> 
> For "order_launch_lcores" function, wmb after that the main lcore updates
> the variable "t->err", 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 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>
> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> ---
>  app/test-eventdev/test_order_common.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> eventdev/test_order_common.c
> index 04456d56db..d7760061ba 100644
> --- a/app/test-eventdev/test_order_common.c
> +++ b/app/test-eventdev/test_order_common.c
> @@ -308,7 +308,6 @@ order_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->err = true;
> -				rte_smp_wmb();
>  				break;
>  			}
>  			old_remaining = remaining;
> --
> 2.25.1
>
Jerin Jacob May 18, 2021, 2:05 p.m. UTC | #2 
On Tue, May 18, 2021 at 2:57 PM Feifei Wang <Feifei.Wang2@arm.com> wrote:
>
> Hi, Jerin
>
> Sorry to disturb you. Would you please help me review this patch, thanks very much.

In slowpath, I thought of having this barrier. But I agree that it can
be removed.
Since this is not bug. We will merge this patch in next release.



>
>
> Best Regards
> Feifei
>
> > -----Original Message-----
> > From: Feifei Wang <feifei.wang2@arm.com>
> > Sent: Monday, May 10, 2021 2:12 PM
> > To: jerinj@marvell.com
> > Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> > <Feifei.Wang2@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Subject: [PATCH] app/eventdev: remove unnecessary barrier for order test
> >
> > For "order_launch_lcores" function, wmb after that the main lcore updates
> > the variable "t->err", 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 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>
> > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > ---
> >  app/test-eventdev/test_order_common.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> > eventdev/test_order_common.c
> > index 04456d56db..d7760061ba 100644
> > --- a/app/test-eventdev/test_order_common.c
> > +++ b/app/test-eventdev/test_order_common.c
> > @@ -308,7 +308,6 @@ order_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->err = true;
> > -                             rte_smp_wmb();
> >                               break;
> >                       }
> >                       old_remaining = remaining;
> > --
> > 2.25.1
> >
>
Feifei Wang May 19, 2021, 2 a.m. UTC | #3 
> -----邮件原件-----
> 发件人: Jerin Jacob <jerinjacobk@gmail.com>
> 发送时间: 2021年5月18日 22:05
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> 抄送: jerinj@marvell.com; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; dev@dpdk.org;
> nd <nd@arm.com>
> 主题: Re: [dpdk-dev] 回复: [PATCH] app/eventdev: remove unnecessary
> barrier for order test
> 
> On Tue, May 18, 2021 at 2:57 PM Feifei Wang <Feifei.Wang2@arm.com>
> wrote:
> >
> > Hi, Jerin
> >
> > Sorry to disturb you. Would you please help me review this patch, thanks
> very much.
> 
> In slowpath, I thought of having this barrier. But I agree that it can be
> removed.
> Since this is not bug. We will merge this patch in next release.

That's Ok. Thanks very much for your reviewing.

Best Regards
Feifei
> 
> 
> 
> >
> >
> > Best Regards
> > Feifei
> >
> > > -----Original Message-----
> > > From: Feifei Wang <feifei.wang2@arm.com>
> > > Sent: Monday, May 10, 2021 2:12 PM
> > > To: jerinj@marvell.com
> > > Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> > > <Feifei.Wang2@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> > > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > > Subject: [PATCH] app/eventdev: remove unnecessary barrier for order
> > > test
> > >
> > > For "order_launch_lcores" function, wmb after that the main lcore
> > > updates the variable "t->err", 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 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>
> > > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > > ---
> > >  app/test-eventdev/test_order_common.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/app/test-eventdev/test_order_common.c b/app/test-
> > > eventdev/test_order_common.c index 04456d56db..d7760061ba 100644
> > > --- a/app/test-eventdev/test_order_common.c
> > > +++ b/app/test-eventdev/test_order_common.c
> > > @@ -308,7 +308,6 @@ order_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->err = true;
> > > -                             rte_smp_wmb();
> > >                               break;
> > >                       }
> > >                       old_remaining = remaining;
> > > --
> > > 2.25.1
> > >
> >
Jerin Jacob June 30, 2021, 6:49 a.m. UTC | #4 
On Mon, May 10, 2021 at 11:42 AM Feifei Wang <feifei.wang2@arm.com> wrote:
>
> For "order_launch_lcores" function, wmb after that the main lcore
> updates the variable "t->err", 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 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>
> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>
Applied to dpdk-next-net-eventdev/for-main. Thanks

> ---
>  app/test-eventdev/test_order_common.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/app/test-eventdev/test_order_common.c b/app/test-eventdev/test_order_common.c
> index 04456d56db..d7760061ba 100644
> --- a/app/test-eventdev/test_order_common.c
> +++ b/app/test-eventdev/test_order_common.c
> @@ -308,7 +308,6 @@ order_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->err = true;
> -                               rte_smp_wmb();
>                                 break;
>                         }
>                         old_remaining = remaining;
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/app/test-eventdev/test_order_common.c b/app/test-eventdev/test_order_common.c
index 04456d56db..d7760061ba 100644
--- a/app/test-eventdev/test_order_common.c
+++ b/app/test-eventdev/test_order_common.c
@@ -308,7 +308,6 @@  order_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->err = true;
-				rte_smp_wmb();
 				break;
 			}
 			old_remaining = remaining;