[dpdk-stable] 回复: [PATCH] examples/eventdev: move eth stop to the end

[Feifei Wang <Feifei.Wang2@arm.com>]

Hi, Van Haaren

> -----邮件原件-----
> 发件人: Van Haaren, Harry <harry.van.haaren@intel.com>
> 发送时间: 2020年12月21日 17:57
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; pbhagavatula@marvell.com;
> stable@dpdk.org; jerinj@marvell.com
> 主题: RE: [PATCH] examples/eventdev: move eth stop to the end
> 
> > -----Original Message-----
> > From: Feifei Wang <feifei.wang2@arm.com>
> > Sent: Monday, December 21, 2020 5:35 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>; Rao, Nikhil
> > <nikhil.rao@intel.com>; Pavan Nikhilesh
> > <pbhagavatula@caviumnetworks.com>
> > Cc: dev@dpdk.org; nd@arm.com; Feifei Wang <feifei.wang2@arm.com>;
> > pbhagavatula@marvell.com; stable@dpdk.org
> > Subject: [PATCH] examples/eventdev: move eth stop to the end
> 
> Suggested title improvement?
> examples/eventdev_pipeline: refactor ethdev port stop
That's OK. I will apply this in the next version.
> 
> > Move eth stop code from "signal_handler" function to the end of "main"
> > function. There are two reasons for this:
> >
> > First, this improves code maintenance and makes code look simple and
> clear.
> > Based on this change, after receiving the interrupt signal, "fdata->done"
> > is set as 1. Then the main thread will wait all worker lcores to jump
> > out of the loop. Finally, the main thread will stop and then close eth dev
> port.
> >
> > Second, for older version, the main thread first stops eth dev port
> > and then waits the end of worker lcore. This may cause errors because
> > it may stop the eth dev port which worker lcores are using. This
> > moving change can fix this by waiting all worker threads to exit and then
> stop the eth dev port.
> 
> I'm OK with the above changes, and agree that moving eth dev port close to
> after lcores return is a worthy change.
> 
> > In the meanwhile, remove wmb in signal_handler.
> >
> > This is because when the main lcore receive the stop signal, it stores
> > 1 into fdata->done. And then the worker lcores load "fdata->done" and
> > jump out of the loop to stop running. Nothing should be stored after
> > updating
> > fdata->done, so the wmb is unnecessary.
> >
> > Fixes: 085edac2ca38 ("examples/eventdev_pipeline: support Tx adapter")
> > Cc: pbhagavatula@marvell.com
> > Cc: stable@dpdk.org
> >
> > Suggested-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > 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>
> 
> +Cc Jerin for Eventdev tree;
> -Cc Pavan's old email address
Thanks very much for this change.
> 
> Ack-ed by: Harry van Haaren <harry.van.haaren@intel.com>

Best Regards
Feifei