From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vaidyanathan Srinivasan Subject: Re: [RFC] Unprepare callback for cpuidle_device Date: Wed, 13 Jul 2011 00:02:00 +0530 Message-ID: <20110712183200.GC6292@dirshya.in.ibm.com> References: <4E15516E.5040209@gmail.com> <82b596dce7b84db206d3afb6da564fb9.squirrel@www.codeaurora.org> <4E170010.2000403@linux.vnet.ibm.com> <473338c2d56302337f83871ce1c201e4.squirrel@www.codeaurora.org> <20110711190856.GC25533@dirshya.in.ibm.com> <57f0d61f9e15b2af262607e872746317.squirrel@www.codeaurora.org> Reply-To: svaidy@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <57f0d61f9e15b2af262607e872746317.squirrel@www.codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: asinghal@codeaurora.org Cc: linux-pm@lists.linux-foundation.org, johlstei@codeaurora.org List-Id: linux-pm@vger.kernel.org * asinghal@codeaurora.org [2011-07-11 14:45:58]: > hello Vaidy, > doesn't tick_nohz_get_sleep_length just return ts->sleep_length > > variable value from the tick_sched structure for that cpu ? Yes, I assumed tick_nohz_get_sleep_length() was going to look for the next timer, but it seems to return the sleep_length that was computed during tick_nohz_stop_sched_tick() as you have mentioned. > The calculation of this sleep_length variable happens in function > > tick_nohz_stop_Sched_tick after call to function get_next_timer_interrupt ?? This makes the implementation more complex. Even if we keep the prepare() and add an unprepare(), all of these will be called within pm_idle() only which is much later than tick_nohz_stop_sched_tick() in cpu_idle(). So it is not easy to have a cpuidle callback before the ts->sleep_length is updated. This leaves us with the option of exploring deferrable hrtimers that will be skipped during idle entry. --Vaidy PS: Please reply inline to keep the discussion well structured. > thanks, > amar > > > > > * asinghal@codeaurora.org [2011-07-11 10:26:56]: > > > >> hello Deepthi, > >> please see my replies inline: > >> > >> > >> > Hi Amar, > >> > > >> > On Friday 08 July 2011 01:22 AM, asinghal@codeaurora.org wrote: > >> >> Hello Trinabh, > >> >> i cannot use the enter callback due to the following > >> >> reason: > >> >> > >> >> the residency calculation(tick)nohz_get_sleep_length) and the idle > >> state > >> >> selection happens in the menu governor. The enter callback is called > >> >> with > >> >> the selected state. > >> > > >> > One would first execute prepare then call select and enter . > >> > So in the newer proposed code, there is no prepare routine. One > >> would > >> > just execute select() and then enter() (Prepare functionality is > >> part of > >> > the > >> > enter routine itself) Is it not possible to cancel the timer, > >> > calculate the execute nohz_get_sleep in enter routine again, > >> > then select the idle state depending on the time . Automatically > >> promote > >> > or demote > >> > idle state based on the latest value returned in the driver ? > >> > > >> Cancelling the timer and calculating the sleep length again in the enter > >> callback is possible; but what does not make sense is calling the select > >> routine again ; definitely the select routine of the governor needs to > >> be > >> called only once. And actually, the sleep length is calculated in the > > > > Can you promote the idle state within the driver based on the new > > information provided by tick_nohz_get_sleep_length() after canceling > > the timer? This will be a hack... but better than calling select() > > again. You are right, we need to design in such a way that select() > > makes the right choice. > > > >> function tick_nohz_stop_sched_tick.( so shld the prepare callback be > >> moved > >> before that in cpu_idle ??) > > > > The prepare callback is in the right place now... if you cancel the > > timer in prepare, then subsequent select() which calls > > tick_nohz_get_sleep_length() will get the correct idle time. Your > > problem is in restarting the timer in case we abort the idle. The > > current place of prepare is fine and more over we are heading in the > > direction to deprecate the prepare itself. > > > >> I am looking for a nice way to cancel and restart the high resolution > >> timers in the idle code; even if that means rearranging the cpu_idle > >> code. > > > > Slight difference, you can cancel and restart within the driver, the > > problem is that you want the ->select() to be executed _after_ > > canceling your hrtimer because it will predict idle much shorter > > otherwise. This is where the cpuidle framework should help. > > > >> Another way i was looking to solve this issue was add deferrable feature > >> to HR timers; if someone has thoughts on that please chime in. > > > > This seems to be a much better idea, where the > > tick_nohz_get_sleep_length() can automatically skip your deferred > > hrtimer with a hope that it will be canceled. We don't have > > a mechanism to actually defer it. > > > > You can try to add deferred flag in hrtimer_mode { } similar to how > > pinned was added. Enhance tick_nohz_get_sleep_length() to skip over > > deferred hrtimer as well. > > > > But the number of users for this feature will be very little to > > justify the complexity. > > > > --Vaidy > > > > > > > >