Re: [dpdk-dev] [PATCH v2 1/2] service: add API to retrieve service core active

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Wednesday, July 22, 2020 7:50 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; David Marchand
> <david.marchand@redhat.com>
> Cc: dev@dpdk.org; igor.romanov@oktetlabs.ru; Yigit, Ferruh
> <ferruh.yigit@intel.com>; nd <nd@arm.com>; aconole@redhat.com;
> l.wojciechow@partner.samsung.com; Phil Yang <Phil.Yang@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Eads, Gage
> <gage.eads@intel.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v2 1/2] service: add API to retrieve service core active
> 
> + Gage (as I referring to his commit below)
> 
> <snip>
> 
> >
> > > > > > +/**
> > > > > > + * Reports if a service lcore is currently running.
> > > > > > + * @retval 0 Service thread is not active, and has been returned to
> > EAL.
> > > > > > + * @retval 1 Service thread is in the service core polling loop.
> > > > > > + * @retval -EINVAL Invalid *lcore_id* provided.
> > > > > > + */
> > > > > > +__rte_experimental
> > > > > > +int32_t rte_service_lcore_active(uint32_t lcore_id);
> > > > > Would 'rte_service_lcore_may_be_active' better? It would be inline
> > > > > with
> > > > 'rte_service_may_be_active'?
> >
> > I think the implementation behind the API is different, so I think _may_be_ is
> > not appropriate for service_lcore_active, keeping same function name for v3.
> >
> > rte_service_lcore_active() checks at a particular point in the calling thread if
> > another thread is active *at that time*. It is either active or not. This is
> > defined, it is deterministic in that the result is either yes or no, and there is
> > no ambiguity at any given check. You're right the value can change *just* after
> > the check - but at the time of the check the answer was deterministic.
> >
> > rte_service_may_be_active() checks if a service *could* be run by a service
> > core. It is not deterministic. A service lcore only sets a service as "active on
> > lcore" (or not active) when it polls it - this opens a window of
> > nondeterministic result. When a runstate is set to off, there is a window of
> > "unknown" before we know certainly that the service is not run on a service
> > core anymore. That is why I believe the _may_be_ is appropriate for this API,
> > it shows this non determinism.
> >
> 
> I am looking at this from the application usage perspective (not the
> implementation). I am pointing to the similarity that exists with the new API. i.e.
> when 'rte_service_lcore_stop' is called, it is not known if the service lcore has
> stopped. If 'rte_service_lcore_active' returns 1, it indicates the lcore 'may be'
> active (the reasoning could be different in this case), it is not guaranteed that it
> is active by the time caller checks it. But when the API returns 0, it guarantees
> that the service lcore (assuming it was started and verified that it was started),
> has stopped.
> 
> Looking at the commit e30dd31847d212cd1b766612cbd980c7d8240baa that
> added the 'rte_service_may_be_active', the use case is a mechanism to identify a
> quiescent state after a service was stopped.
> The use case for the new API is also the same. We want to identify a quiescent
> state after a service lcore is stopped.
> 
> <snip>

Sure - if you feel strongly that we need the _may_be_ to focus the end-users
attention that this is a cross-thread check and has some quiescent property, lets
add it to err on the side of obvious. Will change API name for v3.

Saw the other feedback on patches too - will incorporate and send ASAP, might need
till tomorrow to get it done.