From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Wodkowski, PawelX" <pawelx.wodkowski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH v2] Change alarm cancel function to
 thread-safe:
Date: Mon, 29 Sep 2014 10:11:38 +0000
Message-ID: <F6F2A6264E145F47A18AB6DF8E87425D12B3A80F@IRSMSX102.ger.corp.intel.com>
References: <20140926114630.GA3930@hmsreliant.think-freely.org>
 <F6F2A6264E145F47A18AB6DF8E87425D12B39EB5@IRSMSX102.ger.corp.intel.com>
 <20140926134014.GB3930@hmsreliant.think-freely.org>
 <F6F2A6264E145F47A18AB6DF8E87425D12B39F15@IRSMSX102.ger.corp.intel.com>
 <20140926150156.GB5619@hmsreliant.think-freely.org>
 <2601191342CEEE43887BDE71AB9772582137D88E@IRSMSX104.ger.corp.intel.com>
 <20140926162134.GE5619@hmsreliant.think-freely.org>
 <2601191342CEEE43887BDE71AB9772582137D95F@IRSMSX104.ger.corp.intel.com>
 <20140926193905.GH5619@hmsreliant.think-freely.org>
 <2601191342CEEE43887BDE71AB9772582138410B@IRSMSX104.ger.corp.intel.com>
 <20140928204754.GC4012@localhost.localdomain>
 <F6F2A6264E145F47A18AB6DF8E87425D12B3A553@IRSMSX102.ger.corp.intel.com>
 <2601191342CEEE43887BDE71AB977258213874C5@IRSMSX104.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Cc: "dev-VfR2kkLFssw@public.gmane.org" <dev-VfR2kkLFssw@public.gmane.org>
To: "Ananyev, Konstantin" <konstantin.ananyev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Neil Horman
 <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <2601191342CEEE43887BDE71AB977258213874C5-kPTMFJFq+rGvNW/NfzhIbrfspsVTdybXVpNB7YpNyf8@public.gmane.org>
Content-Language: en-US
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

> >
> > 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 ho=
w those
> > application will behave? Silently changing returned value domain is evi=
l in its
> > pure form.
>=20
> As I can see the impact is very limited.

It is small impact to DPDK but can be huge to user application:
Ex:
If someone use this kind of expression in callback (skipping user app seria=
lization part):
callback () {
...
some_simple_semaphore +=3D rte_alarm_cancel(...));
...
}

Anywhere in the code:
...
If (some_simple_semapore) {
	some_simple_semapore --;
	if (rte_eal_alarm_set(...) !=3D 0)
		some_simple_semapore ++;
}
...

1. Do you notice the change in cancel function?
2. How many hours you spend to find this issue in case of big app/system?

> Only code that does check for (rte_alarm_cancel(...) =3D=3D 0/ !=3D 0) in=
side alarm
> callback function might be affected.
> From other side, indeed, there could exist situations, when the caller ne=
eds to
> know
> was the alarm successfully cancelled or not.
> And if not by what reason.
>=20

I can extend API of rte alarms to add alarm state checking in next patch,  =
but for=20
now, since this is not urgent I think original patch  v2 should be enough.

Pawel