From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751701AbbCQD3a (ORCPT ); Mon, 16 Mar 2015 23:29:30 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:34386 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbbCQD32 (ORCPT ); Mon, 16 Mar 2015 23:29:28 -0400 Message-ID: <55079F87.50906@linux.vnet.ibm.com> Date: Tue, 17 Mar 2015 08:59:11 +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 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: 15031703-0013-0000-0000-0000003111A8 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. It is a normal clock event. In the above context, this hrtimer is being moved from CPUx to the CPU executing that code. Hence it needs to be enqueued onto the new CPU. Any hrtimer enqueue calls into tracing. A hrtimer_reprogram() alone will not suffice. Moreover hrtimer_reprogram() cannot be called directly, can it? nor is it safe. Or am I missing your point ? > > Sure; it might be that for power you only end up with that broadcast > crap enabled on idle/hotplug, but is this always so? The hrtimer broadcast framework gets invoked only during idle. This is platform agnostic. Regards Preeti U Murthy >