Re: [dpdk-dev] [PATCH v2] Change alarm cancel function to thread-safe:

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Monday, September 29, 2014 7:41 AM
> To: Neil Horman; Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] Change alarm cancel function to thread-safe:
> 
> > Yes, this is my concern exactly.
> >
> > >  If that's so, then I suppose we can do: make alarm_cancel() to return a
> > negative value for the case #3 (-EINPROGRESS or something).
> > >  Something like:
> > > ...
> > > if (ap->executing == 0) {
> > >    LIST_REMOVE(ap,next);
> > >     rte_free(ap);
> > >     count++;
> > >     ap = ap_prev;
> > > } else if (pthread_equal(ap->executing_id, pthread_self()) == 0) {
> > >     executing++;
> > > } else {
> > >    ret = -EINPROGRESS;
> > > }
> > > ...
> > > return ((ret != 0) ? ret : count);
> > >
> > > So the return value  will be > 0 for #1, 0 for #2, <0 for #3.
> > > As I remember, you already suggested something similar in one of the previous
> > mails.
> > Yes, I rolled the API changes I suggested in with this model, because I wanted
> > to be able to do precise specification of a timer instance to cancel, but if
> > we're not ready to make that change, I think what you propose above would be
> > suffficient.  Theres some question as to weather we would cancel timers that
> > are
> > still pending on a return of -EINPROGRESS, but I think if we document it
> > accordingly, then it can be worked out just fine.
> >
> > Best
> > Neil
> >
> 
> Image how you will be damned by someone that not even notice you change
> and he Is managing some kind of resource based on returned number of
> set/canceled timers. If you suddenly start returning negative values how those
> application will behave? Silently changing returned value domain is evil in its
> pure form.

As I can see the impact is very limited.
Only code that does check for (rte_alarm_cancel(...) == 0/ != 0) inside alarm callback function might be affected. 
>From other side, indeed, there could exist situations, when the caller needs to know
was the alarm successfully cancelled or not. 
And if not by what reason. 

> 
> From my point of view, problem is virtual because this is user application task to
> know what it can and what it not. If you really want to inform user application
> about timer state you can introduce API call which will interrogate timers list
> and return appropriate value, but for god sake, do not introduce untraceable bugs.
> 
> Pawel