diff mbox

timers: fix off by one tick in timer expiration processing

Message ID 1476393524-113801-1-git-send-email-brian.brooks@linaro.org
State Accepted
Commit ce4fec46c1f6821afb5d0ef9c3099cd094153fd9
Headers show

Commit Message

Brian Brooks Oct. 13, 2016, 9:18 p.m. UTC
A timer pool's tick starts at t0 (zero). Once the first period has passed,
the timer pool is scanned for any timers that have expired since t0 + 1.

Current code does an atomic fetch increment on the tick, but uses the
previous tick during timer expiration processing. What is needed is the
previous tick + 1.

The observable effect without this patch is that timers are expired one tick
period (timer resolution) later than they should be.

Signed-off-by: Brian Brooks <brian.brooks@linaro.org>

---
 platform/linux-generic/odp_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.7.4

Comments

Bill Fischofer Oct. 13, 2016, 9:46 p.m. UTC | #1
Since this is a bug fix, please open a Bug for it so that this can be
tracked as a defect closure.

On Thu, Oct 13, 2016 at 4:18 PM, Brian Brooks <brian.brooks@linaro.org>
wrote:

> A timer pool's tick starts at t0 (zero). Once the first period has passed,

> the timer pool is scanned for any timers that have expired since t0 + 1.

>

> Current code does an atomic fetch increment on the tick, but uses the

> previous tick during timer expiration processing. What is needed is the

> previous tick + 1.

>

> The observable effect without this patch is that timers are expired one

> tick

> period (timer resolution) later than they should be.

>

> Signed-off-by: Brian Brooks <brian.brooks@linaro.org>

>


Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org>



> ---

>  platform/linux-generic/odp_timer.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/platform/linux-generic/odp_timer.c

> b/platform/linux-generic/odp_timer.c

> index becea9d..b26ac6b 100644

> --- a/platform/linux-generic/odp_timer.c

> +++ b/platform/linux-generic/odp_timer.c

> @@ -691,7 +691,7 @@ static void timer_notify(odp_timer_pool *tp)

>         prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);

>

>         /* Scan timer array, looking for timers to expire */

> -       (void)odp_timer_pool_expire(tp, prev_tick);

> +       (void)odp_timer_pool_expire(tp, prev_tick + 1);

>

>         /* Else skip scan of timers. cur_tick was updated and next itimer

>          * invocation will process older expiration ticks as well */

> --

> 2.7.4

>

>
Mike Holmes Oct. 14, 2016, 11:45 a.m. UTC | #2
also need to put
Fixes https://bugs.linaro.org/show_bug.cgi?id=2552

Into the patch description, maybe that can happen as it is pushed ?

On 13 October 2016 at 17:46, Bill Fischofer <bill.fischofer@linaro.org>
wrote:

> Since this is a bug fix, please open a Bug for it so that this can be

> tracked as a defect closure.

>

> On Thu, Oct 13, 2016 at 4:18 PM, Brian Brooks <brian.brooks@linaro.org>

> wrote:

>

> > A timer pool's tick starts at t0 (zero). Once the first period has

> passed,

> > the timer pool is scanned for any timers that have expired since t0 + 1.

> >

> > Current code does an atomic fetch increment on the tick, but uses the

> > previous tick during timer expiration processing. What is needed is the

> > previous tick + 1.

> >

> > The observable effect without this patch is that timers are expired one

> > tick

> > period (timer resolution) later than they should be.

> >

> > Signed-off-by: Brian Brooks <brian.brooks@linaro.org>

> >

>

> Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org>

>

>

> > ---

> >  platform/linux-generic/odp_timer.c | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> >

> > diff --git a/platform/linux-generic/odp_timer.c

> > b/platform/linux-generic/odp_timer.c

> > index becea9d..b26ac6b 100644

> > --- a/platform/linux-generic/odp_timer.c

> > +++ b/platform/linux-generic/odp_timer.c

> > @@ -691,7 +691,7 @@ static void timer_notify(odp_timer_pool *tp)

> >         prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);

> >

> >         /* Scan timer array, looking for timers to expire */

> > -       (void)odp_timer_pool_expire(tp, prev_tick);

> > +       (void)odp_timer_pool_expire(tp, prev_tick + 1);

> >

> >         /* Else skip scan of timers. cur_tick was updated and next itimer

> >          * invocation will process older expiration ticks as well */

> > --

> > 2.7.4

> >

> >

>




-- 
Mike Holmes
Program Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
Maxim Uvarov Oct. 17, 2016, 1:24 p.m. UTC | #3
Merged.

On 10/14/16 14:45, Mike Holmes wrote:
> also need to put

> Fixes https://bugs.linaro.org/show_bug.cgi?id=2552


I put this link to git log.

Maxim.

> Into the patch description, maybe that can happen as it is pushed ?

>

> On 13 October 2016 at 17:46, Bill Fischofer <bill.fischofer@linaro.org>

> wrote:

>

>> Since this is a bug fix, please open a Bug for it so that this can be

>> tracked as a defect closure.

>>

>> On Thu, Oct 13, 2016 at 4:18 PM, Brian Brooks <brian.brooks@linaro.org>

>> wrote:

>>

>>> A timer pool's tick starts at t0 (zero). Once the first period has

>> passed,

>>> the timer pool is scanned for any timers that have expired since t0 + 1.

>>>

>>> Current code does an atomic fetch increment on the tick, but uses the

>>> previous tick during timer expiration processing. What is needed is the

>>> previous tick + 1.

>>>

>>> The observable effect without this patch is that timers are expired one

>>> tick

>>> period (timer resolution) later than they should be.

>>>

>>> Signed-off-by: Brian Brooks <brian.brooks@linaro.org>

>>>

>> Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org>

>>

>>

>>> ---

>>>   platform/linux-generic/odp_timer.c | 2 +-

>>>   1 file changed, 1 insertion(+), 1 deletion(-)

>>>

>>> diff --git a/platform/linux-generic/odp_timer.c

>>> b/platform/linux-generic/odp_timer.c

>>> index becea9d..b26ac6b 100644

>>> --- a/platform/linux-generic/odp_timer.c

>>> +++ b/platform/linux-generic/odp_timer.c

>>> @@ -691,7 +691,7 @@ static void timer_notify(odp_timer_pool *tp)

>>>          prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);

>>>

>>>          /* Scan timer array, looking for timers to expire */

>>> -       (void)odp_timer_pool_expire(tp, prev_tick);

>>> +       (void)odp_timer_pool_expire(tp, prev_tick + 1);

>>>

>>>          /* Else skip scan of timers. cur_tick was updated and next itimer

>>>           * invocation will process older expiration ticks as well */

>>> --

>>> 2.7.4

>>>

>>>

>

>
diff mbox

Patch

diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index becea9d..b26ac6b 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -691,7 +691,7 @@  static void timer_notify(odp_timer_pool *tp)
 	prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);
 
 	/* Scan timer array, looking for timers to expire */
-	(void)odp_timer_pool_expire(tp, prev_tick);
+	(void)odp_timer_pool_expire(tp, prev_tick + 1);
 
 	/* Else skip scan of timers. cur_tick was updated and next itimer
 	 * invocation will process older expiration ticks as well */