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

["Wodkowski, PawelX" <pawelx.wodkowski@intel.com>]

Paweł

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Monday, September 29, 2014 12:33
> To: Wodkowski, PawelX
> Cc: Ananyev, Konstantin; Neil Horman; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] Change alarm cancel function to thread-
> safe:
> 
> On Mon, Sep 29, 2014 at 10:11:38AM +0000, Wodkowski, PawelX wrote:
> > > >
> > > > 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.
> >
> > It is small impact to DPDK but can be huge to user application:
> 
> This is why we traditionally have in the release-notes for each release a
> section dedicated to calling out changes from one release to another. [See
> http://dpdk.org/doc/intel/dpdk-release-notes-1.7.0.pdf section 5]. Since
> from release-to-release there are generally only a couple of changes -
> though our next release may be a little different - the actual changes are
> clear enough to read about without wading through pages of documentation. I
> thinking calling out the change in both the release notes and the API docs
> is sufficient even for a change like this.
> 
> Basically, I wouldn't let API stability factor in too much in trying to get
> a proper fix for this issue.
> 
> /Bruce
> 

Summarizing all proposed solutions and to be able to produce final patch - what
Is desired behavior after fix?

1. do we leave as is in Patch v2:
1.1 if canceling from other thread - if one of the alarms is executing, wait to 
  finish its execution and then cancel any rearmed alarms.
1.2 if canceling from alarm handler and one of the alarms to cancel is this 
  executing callback do no wait for it to finish and cancel anything else.
 
 in both cases return number of canceled callbacks.

2. Do exactly like in 1. but return -EINPROGRESS instead of canceled alarms
  if one of the alarms to cancel is currently executing callback from alarm thread
  (information about number of canceled alarms will be lost).

3. refuse to cancel anything if canceling currently executing alarm from alarm 
  callback and return -EINPROGRESS otherwise do like in 1.1.

4. Implement behaviour 1/2/3 (which?) and add API call to interrogate list of
  alarms and retrun state of given alarms(s).

5. other solutions?

Pawel