Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753206AbbELNww (ORCPT ); Tue, 12 May 2015 09:52:52 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:43001 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751302AbbELNwt (ORCPT ); Tue, 12 May 2015 09:52:49 -0400 Message-ID: <55520589.9080401@oracle.com> Date: Tue, 12 May 2015 09:52:09 -0400 From: Sasha Levin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 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 Subject: Re: [tip:timers/core] hrtimer: Allow concurrent hrtimer_start() for self restarting timers References: <20150415113105.GT5029@twins.programming.kicks-ass.net> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5393 Lines: 107 On 04/22/2015 03:15 PM, tip-bot for Peter Zijlstra wrote: > 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; Hey Peter, I seem to be hitting this warning with the latest -next (2015-05-11): [3344291.055800] WARNING: CPU: 0 PID: 9422 at kernel/time/hrtimer.c:802 hrtimer_forward+0x1f9/0x330() [3344291.057883] Modules linked in: [3344291.058734] CPU: 0 PID: 9422 Comm: trinity-main Tainted: G W 4.1.0-rc3-next-20150511-sa sha-00037-g87c65d4-dirty #2199 [3344291.061763] ffff880003a88000 00000000d4a58de9 ffff880021007c88 ffffffffabc833ac [3344291.063726] 0000000000000000 0000000000000000 ffff880021007cd8 ffffffffa21f0a86 [3344291.065656] ffffffffae6936c8 ffffffffa2386319 ffff880021007cb8 ffff8800211f5f90 [3344291.067590] Call Trace: [3344291.068085] [] dump_stack+0x4f/0x7b [3344291.068949] [] warn_slowpath_common+0xc6/0x120 [3344291.070382] [] warn_slowpath_null+0x1a/0x20 [3344291.071078] [] hrtimer_forward+0x1f9/0x330 [3344291.071834] [] perf_mux_hrtimer_handler+0x340/0x750 [3344291.072616] [] __hrtimer_run_queues+0x30c/0x10d0 [3344291.075592] [] hrtimer_interrupt+0x19c/0x480 [3344291.076340] [] local_apic_timer_interrupt+0x6f/0xc0 [3344291.077156] [] smp_trace_apic_timer_interrupt+0xe3/0x859 [3344291.077968] [] trace_apic_timer_interrupt+0x70/0x80 [3344291.078729] [] ? arch_local_irq_restore+0x10/0x30 [3344291.079582] [] __slab_alloc+0x704/0x790 [3344291.082482] [] kmem_cache_alloc+0x2a4/0x3a0 [3344291.083944] [] proc_alloc_inode+0x1d/0x270 [3344291.084631] [] alloc_inode+0x5b/0x170 [3344291.085278] [] new_inode_pseudo+0x11/0xd0 [3344291.085950] [] proc_get_inode+0x18/0x580 [3344291.086615] [] proc_lookup_de+0xc6/0x160 [3344291.088717] [] proc_tgid_net_lookup+0x5c/0xa0 [3344291.089435] [] lookup_real+0x90/0x120 [3344291.090071] [] __lookup_hash+0xe3/0x100 [3344291.092954] [] walk_component+0x697/0xc10 [3344291.096353] [] path_lookupat+0x13f/0x380 [3344291.097727] [] filename_lookup+0x131/0x1f0 [3344291.098411] [] user_path_at_empty+0xc1/0x160 [3344291.101545] [] user_path_at+0x11/0x20 [3344291.102274] [] vfs_fstatat+0xb1/0x140 [3344291.105842] [] SYSC_newfstatat+0x79/0xd0 [3344291.108737] [] SyS_newfstatat+0xe/0x10 [3344291.109377] [] tracesys_phase2+0x88/0x8d Thanks, Sasha -- 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/