Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755796AbbDOJzi (ORCPT ); Wed, 15 Apr 2015 05:55:38 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:50153 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755654AbbDOJzS (ORCPT ); Wed, 15 Apr 2015 05:55:18 -0400 Message-Id: <20150415095011.743749536@infradead.org> User-Agent: quilt/0.61-1 Date: Wed, 15 Apr 2015 11:41:56 +0200 From: Peter Zijlstra To: tglx@linutronix.de Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, Ben Segall , Roman Gushchin , Paul Turner , Peter Zijlstra Subject: [PATCH 1/3] hrtimer: Fix race between hrtimer_start() and __run_hrtimer() References: <20150415094155.601987867@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline; filename=peterz-hrtimer-hrtimer_start-vs-function.patch Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2417 Lines: 66 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. Fixes: 2d44ae4d7135 ("hrtimer: clean up cpu->base locking tricks") Cc: Ben Segall Cc: Roman Gushchin Cc: Thomas Gleixner Cc: Paul Turner Signed-off-by: Peter Zijlstra (Intel) --- kernel/time/hrtimer.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -791,6 +791,9 @@ u64 hrtimer_forward(struct hrtimer *time if (delta.tv64 < 0) return 0; + if (WARN_ON(timer->state & HRTIMER_STATE_ENQUEUED)) + return 0; + if (interval.tv64 < hrtimer_resolution) interval.tv64 = hrtimer_resolution; @@ -1131,11 +1134,14 @@ static void __run_hrtimer(struct hrtimer * Note: We clear the CALLBACK bit after enqueue_hrtimer and * we do not reprogramm the event hardware. Happens either in * hrtimer_start_range_ns() or in hrtimer_interrupt() + * + * Note: Because we dropped the cpu_base->lock above, + * hrtimer_start_range_ns() can have popped in and enqueued the timer + * for us already. */ - if (restart != HRTIMER_NORESTART) { - BUG_ON(timer->state != HRTIMER_STATE_CALLBACK); + if (restart != HRTIMER_NORESTART && + !(timer->state & HRTIMER_STATE_ENQUEUED)) enqueue_hrtimer(timer, base); - } WARN_ON_ONCE(!(timer->state & HRTIMER_STATE_CALLBACK)); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/