[dpdk-dev] [PATCH v6 08/23] eventtimer: add adapter start/stop definitions

Carrillo, Erik G erik.g.carrillo at intel.com
Fri Jan 19 00:57:20 CET 2018



> -----Original Message-----
> From: Pavan Nikhilesh [mailto:pbhagavatula at caviumnetworks.com]
> Sent: Thursday, January 11, 2018 11:29 AM
> To: Carrillo, Erik G <erik.g.carrillo at intel.com>;
> jerin.jacob at caviumnetworks.com; nipun.gupta at nxp.com;
> hemant.agrawal at nxp.com
> Cc: dev at dpdk.org
> Subject: Re: [PATCH v6 08/23] eventtimer: add adapter start/stop definitions
> 
> On Wed, Jan 10, 2018 at 06:20:59PM -0600, Erik Gabriel Carrillo wrote:
> > Add definitions to the default software implementation for the
> > functions that start and stop adapter instances.
> >
> > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo at intel.com>
> > ---
> >  lib/librte_eventdev/rte_event_timer_adapter.c | 19
> > +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c
> > b/lib/librte_eventdev/rte_event_timer_adapter.c
> > index 38f4dcf..27e6226 100644
> > --- a/lib/librte_eventdev/rte_event_timer_adapter.c
> > +++ b/lib/librte_eventdev/rte_event_timer_adapter.c
> > @@ -565,7 +565,18 @@ sw_event_timer_adapter_uninit(struct
> > rte_event_timer_adapter *adapter)  static int
> > sw_event_timer_adapter_start(const struct rte_event_timer_adapter
> > *adapter)  {
> > -	RTE_SET_USED(adapter);
> > +	int ret;
> > +	struct rte_event_timer_adapter_sw_data *sw_data;
> > +
> > +	sw_data = adapter->data->adapter_priv;
> > +
> > +	ret = rte_service_component_runstate_set(sw_data->service_id, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* If no service core is mapped to the service, fail */
> > +	if (!rte_service_runstate_get(sw_data->service_id))
> > +		return -ENOENT;
> 
> If a service is mapped to more than one service core then the service is
> executed in parallel if it is multi thread safe else every core takes a lock and
> executes the service callback.
> 
> Now in case of timer adapter the following piece of code arms the timer on
> the service core that currently runs the service.
> 	rte_timer_reset_sync(tim, cycles, SINGLE,
> 				     rte_lcore_id(),
> 				     sw_event_timer_cb,
> 				     msg->evtim);
> This might lead to delay in timer expiry being called as rte_timer_manage()
> has to be scheduled on the same service core again.
> 
> The immediate solution that comes to my mind is to limit the number of
> service cores mapped to 1.
> 
> Thoughts?

Yes, good catch.  This service is not MT-safe and if it gets mapped to multiple lcores,  each such lcore will be unable to run the service function while other cores are running it, which would introduce delays unnecessarily since rte_timer_manage gets called less frequently.   I agree that the number of mapped service cores should be limited to 1 while the service is MT-unsafe.  If we do that, we can also put the message ring into single-consumer mode.

In a future change, I can also look at making the service MT-safe.

> 
> Cheers,
> Pavan.
> 
> >
> >  	return 0;
> >  }
> > @@ -573,9 +584,9 @@ sw_event_timer_adapter_start(const struct
> > rte_event_timer_adapter *adapter)  static int
> > sw_event_timer_adapter_stop(const struct rte_event_timer_adapter
> > *adapter)  {
> > -	RTE_SET_USED(adapter);
> > -
> > -	return 0;
> > +	struct rte_event_timer_adapter_sw_data *sw_data;
> > +	sw_data = adapter->data->adapter_priv;
> > +	return rte_service_component_runstate_set(sw_data->service_id,
> 0);
> >  }
> >
> >  static void
> > --
> > 2.6.4
> >


More information about the dev mailing list