Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755224AbbFCVNq (ORCPT ); Wed, 3 Jun 2015 17:13:46 -0400 Received: from casper.infradead.org ([85.118.1.10]:39723 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755461AbbFCVNc (ORCPT ); Wed, 3 Jun 2015 17:13:32 -0400 Date: Wed, 3 Jun 2015 23:13:24 +0200 From: Peter Zijlstra To: Kirill Tkhai Cc: "umgwanakikbuti@gmail.com" , "mingo@elte.hu" , "ktkhai@parallels.com" , "rostedt@goodmis.org" , "tglx@linutronix.de" , "juri.lelli@gmail.com" , "pang.xunlei@linaro.org" , "oleg@redhat.com" , "wanpeng.li@linux.intel.com" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 8/9] hrtimer: Allow hrtimer::function() to free the timer Message-ID: <20150603211324.GC3644@twins.programming.kicks-ass.net> References: <20150603132903.203333087@infradead.org> <20150603134023.156059118@infradead.org> <214021433348760@web25g.yandex.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <214021433348760@web25g.yandex.ru> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3004 Lines: 100 On Wed, Jun 03, 2015 at 07:26:00PM +0300, Kirill Tkhai wrote: > > @@ -402,7 +394,8 @@ extern u64 hrtimer_get_next_event(void); > > ??*/ > > ?static inline int hrtimer_active(const struct hrtimer *timer) > > ?{ > > - return timer->state != HRTIMER_STATE_INACTIVE; > > + return timer->state != HRTIMER_STATE_INACTIVE || > > + timer->base->running == timer; > > ?} > > It seems to be not good, because hrtimer_active() check stops > to be atomic. So the things like hrtimer_try_to_cancel() race > with a callback of self-rearming timer and may return a false > positive result. Hurm.. the below isn't really pretty either I suppose. The best I can say about it is that's its not too expensive on x86. I should probably go sleep.. --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -391,11 +391,25 @@ extern u64 hrtimer_get_next_event(void); * A timer is active, when it is enqueued into the rbtree or the * callback function is running or it's in the state of being migrated * to another cpu. + * + * See __run_hrtimer(). */ -static inline int hrtimer_active(const struct hrtimer *timer) +static inline bool hrtimer_active(const struct hrtimer *timer) { - return timer->state != HRTIMER_STATE_INACTIVE || - timer->base->running == timer; + if (timer->state != HRTIMER_STATE_INACTIVE) + return true; + + smp_rmb(); /* C matches A */ + + if (timer->base->running == timer) + return true; + + smp_rmb(); /* D matches B */ + + if (timer->state != HRTIMER_STATE_INACTIVE) + return true; + + return false; } /* --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -1122,6 +1122,20 @@ static void __run_hrtimer(struct hrtimer debug_deactivate(timer); base->running = timer; + + /* + * Pairs with hrtimer_active(). + * + * [S] base->running = timer [L] timer->state + * WMB RMB + * [S] timer->state = INACTIVE [L] base->running + * + * BUG_ON(base->running != timer && timer->state != INACTIVE) + * + * If we observe INACTIVE we must observe base->running == timer. + */ + smp_wmb(); /* A matches C */ + __remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0); timer_stats_account_hrtimer(timer); fn = timer->function; @@ -1150,6 +1164,20 @@ static void __run_hrtimer(struct hrtimer !(timer->state & HRTIMER_STATE_ENQUEUED)) enqueue_hrtimer(timer, base); + /* + * Pairs with hrtimer_active(). + * + * [S] timer->state = ENQUEUED [L] base->running + * WMB RMB + * [S] base->running = NULL [L] timer->state + * + * BUG_ON(base->running == NULL && timer->state == INACTIVE) + * + * If we observe base->running == NULL, we must observe any preceding + * enqueue. + */ + smp_wmb(); /* B matches D */ + WARN_ON_ONCE(base->running != timer); base->running = NULL; } -- 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/