From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934600AbbDVTPr (ORCPT ); Wed, 22 Apr 2015 15:15:47 -0400 Received: from terminus.zytor.com ([198.137.202.10]:41784 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934383AbbDVTPn (ORCPT ); Wed, 22 Apr 2015 15:15:43 -0400 Date: Wed, 22 Apr 2015 12:15:13 -0700 From: tip-bot for Peter Zijlstra Message-ID: Cc: pjt@google.com, tglx@linutronix.de, linux-kernel@vger.kernel.org, peterz@infradead.org, bsegall@google.com, hpa@zytor.com, mingo@kernel.org, klamm@yandex-team.ru Reply-To: linux-kernel@vger.kernel.org, pjt@google.com, tglx@linutronix.de, klamm@yandex-team.ru, mingo@kernel.org, bsegall@google.com, peterz@infradead.org, hpa@zytor.com In-Reply-To: <20150415113105.GT5029@twins.programming.kicks-ass.net> References: <20150415113105.GT5029@twins.programming.kicks-ass.net> To: linux-tip-commits@vger.kernel.org Subject: [tip:timers/core] hrtimer: Allow concurrent hrtimer_start() for self restarting timers Git-Commit-ID: 5de2755c8c8b3a6b8414870e2c284914a2b42e4d X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit-ID: 5de2755c8c8b3a6b8414870e2c284914a2b42e4d Gitweb: http://git.kernel.org/tip/5de2755c8c8b3a6b8414870e2c284914a2b42e4d Author: Peter Zijlstra AuthorDate: Tue, 20 May 2014 15:49:48 +0200 Committer: Thomas Gleixner CommitDate: Wed, 22 Apr 2015 17:06:52 +0200 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) Cc: Ben Segall Cc: Roman Gushchin Cc: Paul Turner Link: http://lkml.kernel.org/r/20150415113105.GT5029@twins.programming.kicks-ass.net Signed-off-by: Thomas Gleixner --- kernel/time/hrtimer.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c index 3bac942..4adf320 100644 --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -799,6 +799,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 < hrtimer_resolution) interval.tv64 = hrtimer_resolution; @@ -1139,11 +1142,14 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, * 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));