Followup to commit dd2261ed45aa ("hrtimer: Protect lockless access to timer->base") lock_hrtimer_base() fetches timer->base without lock exclusion. Compiler is allowed to read timer->base twice (even if considered dumb) which could end up trying to lock migration_base and return &migration_base. base = timer->base; if (likely(base != &migration_base)) { /* compiler reads timer->base again, and now (base == &migration_base) raw_spin_lock_irqsave(&base->cpu_base->lock, *flags); if (likely(base == timer->base)) return base; /* == &migration_base ! */ Similarly the write sides must use WRITE_ONCE() to avoid store tearing.
Unified · Split
| @@ -164,7 +164,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, · 163 unmodified lines | |||
| 164 | struct hrtimer_clock_base *base; | 164 | struct hrtimer_clock_base *base; |
| 165 | 165 | ||
| 166 | for (;;) { | 166 | for (;;) { |
| 167 | base = timer->base; | 167 | base = READ_ONCE(timer->base); |
| 168 | if (likely(base != &migration_base)) { | 168 | if (likely(base != &migration_base)) { |
| 169 | raw_spin_lock_irqsave(&base->cpu_base->lock, *flags); | 169 | raw_spin_lock_irqsave(&base->cpu_base->lock, *flags); |
| 170 | if (likely(base == timer->base)) | 170 | if (likely(base == timer->base)) |
| @@ -244,7 +244,7 @@ again: · 73 unmodified lines | |||
| 244 | return base; | 244 | return base; |
| 245 | 245 | ||
| 246 | /* See the comment in lock_hrtimer_base() */ | 246 | /* See the comment in lock_hrtimer_base() */ |
| 247 | timer->base = &migration_base; | 247 | WRITE_ONCE(timer->base, &migration_base); |
| 248 | raw_spin_unlock(&base->cpu_base->lock); | 248 | raw_spin_unlock(&base->cpu_base->lock); |
| 249 | raw_spin_lock(&new_base->cpu_base->lock); | 249 | raw_spin_lock(&new_base->cpu_base->lock); |
| 250 | 250 | ||
| @@ -253,10 +253,10 @@ again: · 2 unmodified lines | |||
| 253 | raw_spin_unlock(&new_base->cpu_base->lock); | 253 | raw_spin_unlock(&new_base->cpu_base->lock); |
| 254 | raw_spin_lock(&base->cpu_base->lock); | 254 | raw_spin_lock(&base->cpu_base->lock); |
| 255 | new_cpu_base = this_cpu_base; | 255 | new_cpu_base = this_cpu_base; |
| 256 | timer->base = base; | 256 | WRITE_ONCE(timer->base, base); |
| 257 | goto again; | 257 | goto again; |
| 258 | } | 258 | } |
| 259 | timer->base = new_base; | 259 | WRITE_ONCE(timer->base, new_base); |
| 260 | } else { | 260 | } else { |
| 261 | if (new_cpu_base != this_cpu_base && | 261 | if (new_cpu_base != this_cpu_base && |
| 262 | hrtimer_check_target(timer, new_base)) { | 262 | hrtimer_check_target(timer, new_base)) { |