From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 9DB947DF7 for ; Fri, 26 Sep 2014 11:45:12 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 26 Sep 2014 02:51:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,603,1406617200"; d="scan'208";a="579385640" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by orsmga001.jf.intel.com with ESMTP; 26 Sep 2014 02:51:01 -0700 Received: from irsmsx154.ger.corp.intel.com (163.33.192.96) by IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 26 Sep 2014 10:49:55 +0100 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.200]) by IRSMSX154.ger.corp.intel.com ([169.254.12.124]) with mapi id 14.03.0195.001; Fri, 26 Sep 2014 10:49:55 +0100 From: "Wodkowski, PawelX" To: "Wodkowski, PawelX" , Neil Horman , "Ananyev, Konstantin" Thread-Topic: [dpdk-dev] [PATCH v2] Change alarm cancel function to thread-safe: Thread-Index: AQHP2MAapayPUkT5UUGj9XqXyFJ3T5wR4sKAgAAPjwCAABZmAIAA6q+AgAA413A= Date: Fri, 26 Sep 2014 09:49:54 +0000 Message-ID: References: <1411649768-8084-1-git-send-email-michalx.k.jastrzebski@intel.com> <20140925150807.GD32725@hmsreliant.think-freely.org> <2601191342CEEE43887BDE71AB977258213769DE@IRSMSX105.ger.corp.intel.com> <20140925172358.GG32725@hmsreliant.think-freely.org> In-Reply-To: Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] Change alarm cancel function to thread-safe: X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Sep 2014 09:45:13 -0000 > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_alarm.c > > b/lib/librte_eal/linuxapp/eal/eal_alarm.c > > index 480f0cb..73b6dc5 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_alarm.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_alarm.c > > @@ -64,6 +64,9 @@ > > #define MS_PER_S 1000 > > #define US_PER_S (US_PER_MS * MS_PER_S) > > > > +#define ALARM_EXECUTING (1 << 0) > > +#define ALARM_CANCELLED (1 << 1) > > + > > struct alarm_entry { > > LIST_ENTRY(alarm_entry) next; > > struct timeval time; > > @@ -107,12 +110,14 @@ eal_alarm_callback(struct rte_intr_handle *hdl > > __rte_unused, > > gettimeofday(&now, NULL) =3D=3D 0 && > > (ap->time.tv_sec < now.tv_sec || (ap->time.tv_sec =3D=3D > > now.tv_sec && > > ap->time.tv_usec <=3D > > now.tv_usec))){ > > - ap->executing =3D 1; > > - rte_spinlock_unlock(&alarm_list_lk); >=20 > Removing unlock here introduce deadlock. I does no spotted unlocking bellow so above is invalid. >=20 > > + ap->executing |=3D ALARM_EXECUTING; > > + if (likely(!(ap->executing & ALARM_CANCELLED)) { > > + rte_spinlock_unlock(&alarm_list_lk); > > > > - ap->cb_fn(ap->cb_arg); > > + ap->cb_fn(ap->cb_arg); > > > > - rte_spinlock_lock(&alarm_list_lk); > > + rte_spinlock_lock(&alarm_list_lk); > > + } > > LIST_REMOVE(ap, next); > > rte_free(ap); > > } > > @@ -209,10 +214,9 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, > > void *cb_arg) > > rte_spinlock_lock(&alarm_list_lk); > > /* remove any matches at the start of the list */ > > while ((ap =3D LIST_FIRST(&alarm_list)) !=3D NULL && > > - cb_fn =3D=3D ap->cb_fn && ap->executing =3D=3D 0 && > > + cb_fn =3D=3D ap->cb_fn && > > (cb_arg =3D=3D (void *)-1 || cb_arg =3D=3D ap->cb_arg)) { > > - LIST_REMOVE(ap, next); > > - rte_free(ap); > > + ap->executing |=3D ALARM_CANCELLED; > > count++; > > } > > ap_prev =3D ap; > > @@ -220,10 +224,9 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, > > void *cb_arg) > > /* now go through list, removing entries not at start */ > > LIST_FOREACH(ap, &alarm_list, next) { > > /* this won't be true first time through */ > > - if (cb_fn =3D=3D ap->cb_fn && ap->executing =3D=3D 0 && > > + if (cb_fn =3D=3D ap->cb_fn && > > (cb_arg =3D=3D (void *)-1 || cb_arg =3D=3D ap->cb_arg)) > > { > > - LIST_REMOVE(ap,next); > > - rte_free(ap); > > + ap->executing |=3D ALARM_CANCELLED; > > count++; > > ap =3D ap_prev; > > } >=20 > Pawel