| From 11aa25d058b13acd04b59064ac0b29e9242dd0a9 Mon Sep 17 00:00:00 2001 |
| From: Li RongQing <lirongqing@baidu.com> |
| Date: Thu, 19 Sep 2019 20:04:47 +0800 |
| Subject: [PATCH] timer: Read jiffies once when forwarding base clk |
| |
| commit e430d802d6a3aaf61bd3ed03d9404888a29b9bf9 upstream. |
| |
| The timer delayed for more than 3 seconds warning was triggered during |
| testing. |
| |
| Workqueue: events_unbound sched_tick_remote |
| RIP: 0010:sched_tick_remote+0xee/0x100 |
| ... |
| Call Trace: |
| process_one_work+0x18c/0x3a0 |
| worker_thread+0x30/0x380 |
| kthread+0x113/0x130 |
| ret_from_fork+0x22/0x40 |
| |
| The reason is that the code in collect_expired_timers() uses jiffies |
| unprotected: |
| |
| if (next_event > jiffies) |
| base->clk = jiffies; |
| |
| As the compiler is allowed to reload the value base->clk can advance |
| between the check and the store and in the worst case advance farther than |
| next event. That causes the timer expiry to be delayed until the wheel |
| pointer wraps around. |
| |
| Convert the code to use READ_ONCE() |
| |
| Fixes: 236968383cf5 ("timers: Optimize collect_expired_timers() for NOHZ") |
| Signed-off-by: Li RongQing <lirongqing@baidu.com> |
| Signed-off-by: Liang ZhiCheng <liangzhicheng@baidu.com> |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Cc: stable@vger.kernel.org |
| Link: https://lkml.kernel.org/r/1568894687-14499-1-git-send-email-lirongqing@baidu.com |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/kernel/time/timer.c b/kernel/time/timer.c |
| index 343c7ba33b1c..7d63b7347066 100644 |
| --- a/kernel/time/timer.c |
| +++ b/kernel/time/timer.c |
| @@ -1593,24 +1593,26 @@ void timer_clear_idle(void) |
| static int collect_expired_timers(struct timer_base *base, |
| struct hlist_head *heads) |
| { |
| + unsigned long now = READ_ONCE(jiffies); |
| + |
| /* |
| * NOHZ optimization. After a long idle sleep we need to forward the |
| * base to current jiffies. Avoid a loop by searching the bitfield for |
| * the next expiring timer. |
| */ |
| - if ((long)(jiffies - base->clk) > 2) { |
| + if ((long)(now - base->clk) > 2) { |
| unsigned long next = __next_timer_interrupt(base); |
| |
| /* |
| * If the next timer is ahead of time forward to current |
| * jiffies, otherwise forward to the next expiry time: |
| */ |
| - if (time_after(next, jiffies)) { |
| + if (time_after(next, now)) { |
| /* |
| * The call site will increment base->clk and then |
| * terminate the expiry loop immediately. |
| */ |
| - base->clk = jiffies; |
| + base->clk = now; |
| return 0; |
| } |
| base->clk = next; |
| -- |
| 2.7.4 |
| |