From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sanford, Robert" Subject: Re: [PATCH] timer: fix break list when timer_cb reset running timer Date: Wed, 20 Jul 2016 20:17:08 +0000 Message-ID: References: <1468778880-25515-1-git-send-email-h.mikita89@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , Thomas Monjalon To: Hiroyuki Mikita Return-path: Received: from prod-mail-xrelay06.akamai.com (prod-mail-xrelay06.akamai.com [96.6.114.98]) by dpdk.org (Postfix) with ESMTP id E40AC37B8 for ; Wed, 20 Jul 2016 22:17:15 +0200 (CEST) In-Reply-To: <1468778880-25515-1-git-send-email-h.mikita89@gmail.com> Content-Language: en-US Content-ID: <6979AD2C8E427546A08A5AA0BFB6480F@akamai.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Hiroyuki, I am reviewing your 3 timer patches. Can you please explain in more detail your use-case that results in a problem? For example, is it when timer A's callback tries to reset (rte_timer_reset) timer B? If yes, is timer B in PENDING state and likely to expire soon? -- Thanks, Robert On 7/17/16 2:08 PM, "Hiroyuki Mikita" wrote: >When timer_cb resets another running timer on the same lcore, >the list of expired timers is chained to the pending-list. >This commit prevents a running timer from being reset >by not its own timer_cb. > >Signed-off-by: Hiroyuki Mikita >--- > lib/librte_timer/rte_timer.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > >diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c >index 3dcdab5..00e80c9 100644 >--- a/lib/librte_timer/rte_timer.c >+++ b/lib/librte_timer/rte_timer.c >@@ -69,6 +69,9 @@ struct priv_timer { >=20 > unsigned prev_lcore; /**< used for lcore round robin */ >=20 >+ /** running timer on this lcore now */ >+ struct rte_timer *running_tim; >+ > #ifdef RTE_LIBRTE_TIMER_DEBUG > /** per-lcore statistics */ > struct rte_timer_debug_stats stats; >@@ -135,9 +138,12 @@ timer_set_config_state(struct rte_timer *tim, > while (success =3D=3D 0) { > prev_status.u32 =3D tim->status.u32; >=20 >- /* timer is running on another core, exit */ >+ /* timer is running on another core >+ * or ready to run on local core, exit >+ */ > if (prev_status.state =3D=3D RTE_TIMER_RUNNING && >- prev_status.owner !=3D (uint16_t)lcore_id) >+ (prev_status.owner !=3D (uint16_t)lcore_id || >+ tim !=3D priv_timer[lcore_id].running_tim)) > return -1; >=20 > /* timer is being configured on another core */ >@@ -580,6 +586,7 @@ void rte_timer_manage(void) > for (tim =3D run_first_tim; tim !=3D NULL; tim =3D next_tim) { > next_tim =3D tim->sl_next[0]; > priv_timer[lcore_id].updated =3D 0; >+ priv_timer[lcore_id].running_tim =3D tim; >=20 > /* execute callback function with list unlocked */ > tim->f(tim, tim->arg); >@@ -610,6 +617,7 @@ void rte_timer_manage(void) > rte_spinlock_unlock(&priv_timer[lcore_id].list_lock); > } > } >+ priv_timer[lcore_id].running_tim =3D NULL; > } >=20 > /* dump statistics about timers */ >--=20 >2.7.4 >