From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965451AbbCPO4V (ORCPT ); Mon, 16 Mar 2015 10:56:21 -0400 Received: from casper.infradead.org ([85.118.1.10]:43472 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932223AbbCPO4S (ORCPT ); Mon, 16 Mar 2015 10:56:18 -0400 Date: Mon, 16 Mar 2015 15:56:10 +0100 From: Peter Zijlstra To: Preeti U Murthy Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org Subject: Re: [PATCH] tick/broadcast-hrtimer : Fix suspicious RCU usage in idle loop Message-ID: <20150316145610.GF21418@twins.programming.kicks-ass.net> References: <20150226032202.20019.91636.stgit@preeti.in.ibm.com> <20150302145354.GM21418@twins.programming.kicks-ass.net> <54F7DD4E.1000100@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54F7DD4E.1000100@linux.vnet.ibm.com> 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 On Thu, Mar 05, 2015 at 10:06:30AM +0530, Preeti U Murthy wrote: > > On 03/02/2015 08:23 PM, Peter Zijlstra wrote: > > On Thu, Feb 26, 2015 at 08:52:02AM +0530, Preeti U Murthy wrote: > >> The hrtimer mode of broadcast queues hrtimers in the idle entry > >> path so as to wakeup cpus in deep idle states. > > > > Callgraph please... > > cpuidle_idle_call() > |____ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, ....)) > |_____tick_broadcast_set_event() > |____clockevents_program_event() > |____bc_set_next() > > > >> hrtimer_{start/cancel} > >> functions call into tracing which uses RCU. But it is not legal to call > >> into RCU in cpuidle because it is one of the quiescent states. Hence > >> protect this region with RCU_NONIDLE which informs RCU that the cpu > >> is momentarily non-idle. > > > > It it not clear to me that every user of bc_set_next() is from IDLE. > > From what I can tell it ends up being clockevents_program_event() and > > that is called quite a lot. > > bc_set_next() is called from at places: > 1. Idle entry : It is called when a cpu in its idle entry path finds the > need to reset the broadcast hrtimer. > 2. CPU offline operations : When the cpu on which the broadcast hrtimer > is being queued goes offline. > > So you see that almost all the time, it is called in idle entry path. How about: hrtimer_reprogram() tick_program_event() clockevents_program_event() ->set_next_ktime() That is called from !idle loads of times. I guess I'm not seeing what avoids &ce_broadcast_hrtimer from being the 'normal' clock event. Sure; it might be that for power you only end up with that broadcast crap enabled on idle/hotplug, but is this always so?