From fec12030a9c63490ef357e71e2430133a64bbe34 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Tue, 14 Feb 2017 22:15:51 -0500 Subject: [PATCH] sam3x8e/timer: Be careful of races in timer_set_next() It's possible for sched_del_timer() to be called on a timer that fires just after sched_del_timer disables irqs but before the next timer is scheduled. In this case be sure to clear the irq pending status flag after scheduling the next timer so that a delayed irq doesn't cause the wrong timer to be run. For the same reason, make sure to check the irq pending status flag at the start of the timer irq. Also, as a safety check, make sure timer_set_next() isn't called from within the timer irq dispatch loop. Signed-off-by: Kevin O'Connor --- src/sam3x8e/timer.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/sam3x8e/timer.c b/src/sam3x8e/timer.c index 0d3728ed..667e9b65 100644 --- a/src/sam3x8e/timer.c +++ b/src/sam3x8e/timer.c @@ -30,9 +30,10 @@ timer_from_us(uint32_t us) void __visible TC0_Handler(void) { - TC0->TC_CHANNEL[0].TC_SR; // clear irq pending irq_disable(); - sched_timer_kick(); + uint32_t status = TC0->TC_CHANNEL[0].TC_SR; // read to clear irq pending + if (likely(status & TC_SR_CPAS)) + sched_timer_kick(); irq_enable(); } @@ -42,6 +43,13 @@ timer_set(uint32_t value) TC0->TC_CHANNEL[0].TC_RA = value; } +static void +timer_set_clear(uint32_t value) +{ + TC0->TC_CHANNEL[0].TC_RA = value; + TC0->TC_CHANNEL[0].TC_SR; // read to clear irq pending +} + static void timer_init(void) { @@ -83,12 +91,16 @@ uint8_t timer_set_next(uint32_t next) { uint32_t cur = timer_read_time(); + if (sched_is_before(TC0->TC_CHANNEL[0].TC_RA, cur) + && !(TC0->TC_CHANNEL[0].TC_SR & TC_SR_CPAS)) + // Already processing timer irqs + try_shutdown("timer_set_next called during timer dispatch"); uint32_t mintime = cur + TIMER_MIN_TICKS; if (sched_is_before(mintime, next)) { - timer_set(next); + timer_set_clear(next); return 0; } - timer_set(mintime); + timer_set_clear(mintime); return 1; }