[RHEL7,COMMIT] ms/hrtimer: Allow concurrent hrtimer_start() for self restarting timers

Submitted by Konstantin Khorenko on Sept. 26, 2018, 3:26 p.m.


Message ID 201809261526.w8QFQSM3003560@finist_ce7.work
State New
Series "ms/hrtimer: Allow concurrent hrtimer_start() for self restarting timers"
Headers show

Commit Message

Konstantin Khorenko Sept. 26, 2018, 3:26 p.m.
The commit is pushed to "branch-rh7-3.10.0-862.11.6.vz7.71.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.11.6.vz7.71.14
commit 6cb4e4592712f59d2575d8f77524b4369c5e6b42
Author: Peter Zijlstra <peterz@infradead.org>
Date:   Wed Sep 26 18:26:28 2018 +0300

    ms/hrtimer: Allow concurrent hrtimer_start() for self restarting timers
    Because we drop cpu_base->lock around calling hrtimer::function, it is
    possible for hrtimer_start() to come in between and enqueue the timer.
    If hrtimer::function then returns HRTIMER_RESTART we'll hit the BUG_ON
    because HRTIMER_STATE_ENQUEUED will be set.
    Since the above is a perfectly valid scenario, remove the BUG_ON and
    make the enqueue_hrtimer() call conditional on the timer not being
    enqueued already.
    NOTE: in that concurrent scenario its entirely common for both sites
    to want to modify the hrtimer, since hrtimers don't provide
    serialization themselves be sure to provide some such that the
    hrtimer::function and the hrtimer_start() caller don't both try and
    fudge the expiration state at the same time.
    To that effect, add a WARN when someone tries to forward an already
    enqueued timer, the most common way to change the expiry of self
    restarting timers. Ideally we'd put the WARN in everything modifying
    the expiry but most of that is inlines and we don't need the bloat.
    Fixes: 2d44ae4d7135 ("hrtimer: clean up cpu->base locking tricks")
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: Ben Segall <bsegall@google.com>
    Cc: Roman Gushchin <klamm@yandex-team.ru>
    Cc: Paul Turner <pjt@google.com>
    Link: http://lkml.kernel.org/r/20150415113105.GT5029@twins.programming.kicks-ass.net
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    (cherry picked from commit 5de2755c8c8b3a6b8414870e2c284914a2b42e4d)
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
    Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
 kernel/hrtimer.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 70c7747aa61a..30bf706bc211 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -843,6 +843,9 @@  u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval)
 	if (delta.tv64 < 0)
 		return 0;
+	if (WARN_ON(timer->state & HRTIMER_STATE_ENQUEUED))
+		return 0;
 	if (interval.tv64 < timer->base->resolution.tv64)
 		interval.tv64 = timer->base->resolution.tv64;
@@ -1222,6 +1225,10 @@  static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 	 * Because we run timers from hardirq context, there is no chance
 	 * they get migrated to another cpu, therefore its safe to unlock
 	 * the timer base.
+	 *
+	 * Note: Because we dropped the cpu_base->lock above,
+	 * hrtimer_start_range_ns() can have popped in and enqueued the timer
+	 * for us already
 	trace_hrtimer_expire_entry(timer, now);
@@ -1234,10 +1241,9 @@  static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 	 * we do not reprogramm the event hardware. Happens either in
 	 * hrtimer_start_range_ns() or in hrtimer_interrupt()
-	if (restart != HRTIMER_NORESTART) {
-		BUG_ON(timer->state != HRTIMER_STATE_CALLBACK);
+	if (restart != HRTIMER_NORESTART &&
+		!(timer->state & HRTIMER_STATE_ENQUEUED))
 		enqueue_hrtimer(timer, base);
-	}