From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752480AbbCQEUK (ORCPT ); Tue, 17 Mar 2015 00:20:10 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:43608 "EHLO e39.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751719AbbCQEUD (ORCPT ); Tue, 17 Mar 2015 00:20:03 -0400 Message-ID: <5507AB61.90907@linux.vnet.ibm.com> Date: Tue, 17 Mar 2015 09:49:45 +0530 From: Preeti U Murthy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Peter Zijlstra CC: tglx@linutronix.de, linux-kernel@vger.kernel.org, Ingo Molnar Subject: Re: [PATCH] tick/broadcast-hrtimer : Fix suspicious RCU usage in idle loop References: <20150226032202.20019.91636.stgit@preeti.in.ibm.com> <20150302145354.GM21418@twins.programming.kicks-ass.net> <54F7DD4E.1000100@linux.vnet.ibm.com> <20150316145610.GF21418@twins.programming.kicks-ass.net> In-Reply-To: <20150316145610.GF21418@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15031704-0033-0000-0000-000003F7FDCB Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/16/2015 08:26 PM, Peter Zijlstra wrote: > 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. Ok I see your point now. Sorry about having misinterpreted it previously. ce_broadcast_hrtimer is not the per-cpu clock device. It is not a real clock device. It is a pseudo clock device, which is called only from the guts of the broadcast framework. When it is programmed, it queues a hrtimer and programs the per-cpu clock device. in the fashion mentioned above. No hrtimer programming/starting/canceling will get routed through bc_set_next(). The broadcast framework makes use of a separate broadcast clock device, which is never the per-cpu clock device to wake cpus from idle. This device is programmed explicitly when required and not indirectly via timer queueing. *Only* when this broadcast clock device needs to reprogrammed, bc_set_next() gets called on those archs which *do not have a real broadcast clock device*. And the whole thing kicks in when cpus go idle only, not just for PowerPC but for ARM as well. Regards Preeti U Murthy > > Sure; it might be that for power you only end up with that broadcast > crap enabled on idle/hotplug, but is this always so? >