timer fixes
Linus,
Please pull the latest timers-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus
# HEAD: 6bad6bccf2d717f652d37e63cf261eaa23466009 timers: Prevent base clock corruption when forwarding
Fix four timer locking races: two were noticed by you while reviewing the code while chasing for a corruption bug, and two from fixing spurious USB timeouts.
Thanks,
Ingo
------------------>
Thomas Gleixner (4):
timers: Plug locking race vs. timer migration
timers: Lock base for same bucket optimization
timers: Prevent base clock rewind when forwarding clock
timers: Prevent base clock corruption when forwarding
kernel/time/timer.c | 74 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 30 deletions(-)
kernel/time/timer.c +44 -30
@@ -878,7 +878,7 @@ static inline struct timer_base *get_timer_base(u32 tflags)#ifdef CONFIG_NO_HZ_COMMON static inline struct timer_base * -__get_target_base (struct timer_base *base,unsigned tflags) +get_target_base (struct timer_base *base,unsigned tflags) {#ifdef CONFIG_SMP if ((tflags &TIMER_PINNED ) || !base->migration_enabled)
@@ -891,25 +891,27 @@ __get_target_base(struct timer_base *base, unsigned tflags)static inline void forward_timer_base (struct timer_base *base) { +unsigned long jnow =READ_ONCE (jiffies); +/* * We only forward the base when it's idle and we have a delta between * base clock and jiffies. */ -if (!base->is_idle || (long ) (jiffies - base->clk) <2 ) +if (!base->is_idle || (long ) (jnow - base->clk) <2 )return ;/* * If the next expiry value is > jiffies, then we fast forward to * jiffies otherwise we forward to the next expiry value. */ -if (time_after (base->next_expiry, jiffies)) - base->clk = jiffies; +if (time_after (base->next_expiry, jnow)) + base->clk = jnow;else base->clk = base->next_expiry; }#else static inline struct timer_base * -__get_target_base (struct timer_base *base,unsigned tflags) +get_target_base (struct timer_base *base,unsigned tflags) {return get_timer_this_cpu_base (tflags); }
@@ -917,14 +919,6 @@ __get_target_base(struct timer_base *base, unsigned tflags)static inline void forward_timer_base (struct timer_base *base) { }#endif -static inline struct timer_base * -get_target_base (struct timer_base *base,unsigned tflags) -{ -struct timer_base *target =__get_target_base (base, tflags); - -forward_timer_base (target); -return target; -}/* * We are using hashed locking: Holding per_cpu(timer_bases[x]).lock means
@@ -943,7 +937,14 @@ static struct timer_base *lock_timer_base(struct timer_list *timer, {for (;;) {struct timer_base *base; -u32 tf = timer->flags; +u32 tf; + +/* +* We need to use READ_ONCE() here, otherwise the compiler +* might re-read @tf between the check for TIMER_MIGRATING +* and spin_lock(). +*/ + tf =READ_ONCE (timer->flags);if (!(tf &TIMER_MIGRATING )) { base =get_timer_base (tf);
@@ -964,6 +965,8 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)unsigned long clk =0 , flags;int ret =0 ; +BUG_ON (!timer->function); +/* * This is a common optimization triggered by the networking code - if * the timer is re-modified to have the same timeout or ends up in the
@@ -972,13 +975,16 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)if (timer_pending (timer)) {if (timer->expires == expires)return 1 ; +/* -* Take the current timer_jiffies of base, but without holding -* the lock! +* We lock timer base and calculate the bucket index right +* here. If the timer ends up in the same bucket, then we +* just update the expiry time and avoid the whole +* dequeue/enqueue dance. */ - base =get_timer_base (timer->flags); - clk = base->clk; + base =lock_timer_base (timer, &flags); + clk = base->clk; idx =calc_wheel_index (expires, clk);/*
@@ -988,14 +994,14 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)*/ if (idx ==timer_get_idx (timer)) { timer->expires = expires; -return 1 ; + ret =1 ; + goto out_unlock; } + }else { + base =lock_timer_base (timer, &flags); }timer_stats_timer_set_start_info (timer); -BUG_ON (!timer->function); - - base =lock_timer_base (timer, &flags); ret =detach_if_pending (timer, base, false);if (!ret && pending_only)
@@ -1025,12 +1031,16 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only) } } +/* Try to forward a stale timer base clock */ +forward_timer_base (base); + timer->expires = expires;/* * If 'idx' was calculated above and the base time did not advance -* between calculating 'idx' and taking the lock, only enqueue_timer() -* and trigger_dyntick_cpu() is required. Otherwise we need to -* (re)calculate the wheel index via internal_add_timer(). +* between calculating 'idx' and possibly switching the base, only +* enqueue_timer() and trigger_dyntick_cpu() is required. Otherwise +* we need to (re)calculate the wheel index via +* internal_add_timer(). */ if (idx !=UINT_MAX && clk == base->clk) {enqueue_timer (base, timer, idx);
@@ -1510,12 +1520,16 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem) is_max_delta = (nextevt == base->clk +NEXT_TIMER_MAX_DELTA ); base->next_expiry = nextevt;/* -* We have a fresh next event. Check whether we can forward the base: +* We have a fresh next event. Check whether we can forward the +* base. We can only do that when @basej is past base->clk +* otherwise we might rewind base->clk. */ -if (time_after (nextevt, jiffies)) - base->clk = jiffies; -else if (time_after (nextevt, base->clk)) - base->clk = nextevt; +if (time_after (basej, base->clk)) { +if (time_after (nextevt, basej)) + base->clk = basej; +else if (time_after (nextevt, base->clk)) + base->clk = nextevt; + }if (time_before_eq (nextevt, basej)) { expires = basem;