app/eventdev: remove unnecessary barrier for order test
Checks
Commit Message
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
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
>
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
> >
>
> -----邮件原件-----
> 发件人: 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
> > >
> >
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
>
@@ -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;