Re: [dpdk-dev] [PATCH v4 2/2] event/sw: support device stop flush callback

["Eads, Gage" <gage.eads@intel.com>]

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, March 23, 2018 12:06 PM
> To: Eads, Gage <gage.eads@intel.com>; dev@dpdk.org
> Cc: jerin.jacob@caviumnetworks.com; hemant.agrawal@nxp.com; Richardson,
> Bruce <bruce.richardson@intel.com>; santosh.shukla@caviumnetworks.com;
> nipun.gupta@nxp.com
> Subject: RE: [PATCH v4 2/2] event/sw: support device stop flush callback
> 
> > From: Eads, Gage
> > Sent: Tuesday, March 20, 2018 2:13 PM
> > To: dev@dpdk.org
> > Cc: jerin.jacob@caviumnetworks.com; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; hemant.agrawal@nxp.com; Richardson,
> > Bruce <bruce.richardson@intel.com>; santosh.shukla@caviumnetworks.com;
> > nipun.gupta@nxp.com
> > Subject: [PATCH v4 2/2] event/sw: support device stop flush callback
> >
> > This commit also adds a flush callback test to the sw eventdev's
> > selftest suite.
> >
> > Signed-off-by: Gage Eads <gage.eads@intel.com>
> > ---
> >  drivers/event/sw/sw_evdev.c          | 25 ++++++++++-
> >  drivers/event/sw/sw_evdev_selftest.c | 80
> > +++++++++++++++++++++++++++++++++++-
> >  2 files changed, 102 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
> > index 0e89f11..11f394f 100644
> > --- a/drivers/event/sw/sw_evdev.c
> > +++ b/drivers/event/sw/sw_evdev.c
> > @@ -362,8 +362,25 @@ sw_init_qid_iqs(struct sw_evdev *sw)  }
> >
> >  static void
> > -sw_clean_qid_iqs(struct sw_evdev *sw)
> > +sw_flush_iq(struct rte_eventdev *dev, struct sw_iq *iq)
> >  {
> > +	struct sw_evdev *sw = sw_pmd_priv(dev);
> > +
> > +	while (iq_count(iq) > 0) {
> > +		struct rte_event event;
> > +
> > +		iq_dequeue_burst(sw, iq, &event, 1);
> > +
> > +		dev->dev_ops->dev_stop_flush(dev->data->dev_id,
> > +					     event,
> > +					     dev->data->dev_stop_flush_arg);
> 
> 
> Adding check that dev_stop_flush() is non-NULL?
> 
> [Update: Ah I see you do this below already. Still, better check twice I think, the
> data path isn't running here anyway in case future me decides to call
> sw_flush_iq() without performing the check]

Agreed, will fix in v5.

> 
> if(dev->dev_ops->dev_stop_flush)
>    dev->dev_ops->dev_stop_flush(...);
> 
> 
> 
> 
> > @@ -702,7 +723,7 @@ static void
> >  sw_stop(struct rte_eventdev *dev)
> >  {
> >  	struct sw_evdev *sw = sw_pmd_priv(dev);
> > -	sw_clean_qid_iqs(sw);
> > +	sw_clean_qid_iqs(dev);
> 
> Based on the port buffers comment on 1/2, we probably need a
> sw_clean_port_buffers(sw); here to return any events in the port owned SW
> buffers?
> 

Agreed, if there is buy-in on the approach discussed in patch #1.