Hi, Van Haaren
> -----邮件原件-----
> 发件人: Van Haaren, Harry <harry.van.haa...@intel.com>
> 发送时间: 2020年12月21日 17:57
> 收件人: Feifei Wang <feifei.wa...@arm.com>; Rao, Nikhil
> <nikhil....@intel.com>
> 抄送: dev@dpdk.org; nd <n...@arm.com>; pbhagavat...@marvell.com;
> sta...@dpdk.org; jer...@marvell.com
> 主题: RE: [PATCH] examples/eventdev: move eth stop to the end
>
> > -----Original Message-----
> > From: Feifei Wang <feifei.wa...@arm.com>
> > Sent: Monday, December 21, 2020 5:35 AM
> > To: Van Haaren, Harry <harry.van.haa...@intel.com>; Rao, Nikhil
> > <nikhil....@intel.com>; Pavan Nikhilesh
> > <pbhagavat...@caviumnetworks.com>
> > Cc: dev@dpdk.org; n...@arm.com; Feifei Wang <feifei.wa...@arm.com>;
> > pbhagavat...@marvell.com; sta...@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: pbhagavat...@marvell.com
> > Cc: sta...@dpdk.org
> >
> > Suggested-by: Ruifeng Wang <ruifeng.w...@arm.com>
> > Signed-off-by: Feifei Wang <feifei.wa...@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@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.haa...@intel.com>
Best Regards
Feifei
