From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH] timer bug fix. Date: Fri, 23 May 2014 16:44:24 +0200 Message-ID: <537F5EC8.70109@6wind.com> References: <1400704555-18818-1-git-send-email-vadim.suraev@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit To: Vadim Suraev , dev-VfR2kkLFssw@public.gmane.org Return-path: In-Reply-To: <1400704555-18818-1-git-send-email-vadim.suraev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Acked-by: Olivier Matz On 05/21/2014 10:35 PM, Vadim Suraev wrote: > Bug: When a timer is running > - if rte_timer_stop is called, the pending decrement is > skipped (decremented only if the timer is pending) and due > to the update flag the future processing is skipped so the > timer is counted as pending while it is stopped. - the same > applies when rte_timer_reset is called but then the pending > statistics is additionally incremented so the timer is > counted pending twice. > Solution: decrement the pending > statistics after returning from the callback. If > rte_timer_stop was called, it skipped decrementing the > pending statistics. If rte_time_reset was called, the > pending statistics was incremented. If neither was called > and the timer is periodic, the pending statistics is > incremented when it is reloaded > > > Signed-off-by: Vadim Suraev > --- > lib/librte_timer/rte_timer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c > index 1ebd223..7035bed 100755 > --- a/lib/librte_timer/rte_timer.c > +++ b/lib/librte_timer/rte_timer.c > @@ -551,7 +551,7 @@ void rte_timer_manage(void) > tim->f(tim, tim->arg); > > rte_spinlock_lock(&priv_timer[lcore_id].list_lock); > - > + __TIMER_STAT_ADD(pending, -1); > /* the timer was stopped or reloaded by the callback > * function, we have nothing to do here */ > if (priv_timer[lcore_id].updated == 1) > @@ -559,7 +559,6 @@ void rte_timer_manage(void) > > if (tim->period == 0) { > /* remove from done list and mark timer as stopped */ > - __TIMER_STAT_ADD(pending, -1); > status.state = RTE_TIMER_STOP; > status.owner = RTE_TIMER_NO_OWNER; > rte_wmb(); > @@ -568,6 +567,7 @@ void rte_timer_manage(void) > else { > /* keep it in list and mark timer as pending */ > status.state = RTE_TIMER_PENDING; > + __TIMER_STAT_ADD(pending, 1); > status.owner = (int16_t)lcore_id; > rte_wmb(); > tim->status.u32 = status.u32; >