* [RFC] Unprepare callback for cpuidle_device @ 2011-07-06 20:23 asinghal 2011-07-07 6:25 ` Trinabh Gupta 0 siblings, 1 reply; 8+ messages in thread From: asinghal @ 2011-07-06 20:23 UTC (permalink / raw) To: linux-pm; +Cc: johlstei We plan to use high resolution timers in one of our modules, with the requirement that we cancel these timers when the cpu goes idle and restart them when the cpu comes out of idle. We are cancelling the timers in cpuidle prepare callback. The problem is that if the need_resched() call in drivers/cpuidle/cpuidle.c returns true, how do we restart the timer? If the call returns false, we can restart the timer in the cpuidle enter callback. The solution to the problem that we have in mind is adding an unprepare callback to the cpuidle_device struct, and calling it if needs_resched() returns true. Another option is to implement deferred timers for hrtimers. Which of the two options is the better solution, or is there another feasible alternative? sincerely, Amar Singhal ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-06 20:23 [RFC] Unprepare callback for cpuidle_device asinghal @ 2011-07-07 6:25 ` Trinabh Gupta 2011-07-07 19:52 ` asinghal 0 siblings, 1 reply; 8+ messages in thread From: Trinabh Gupta @ 2011-07-07 6:25 UTC (permalink / raw) To: asinghal; +Cc: linux-pm, johlstei On 07/06/2011 04:23 PM, asinghal@codeaurora.org wrote: > We plan to use high resolution timers in one of our modules, with the > requirement that we cancel these timers when the cpu goes idle and restart > them when the cpu comes out of idle. > > We are cancelling the timers in cpuidle prepare callback. The problem is > that if the need_resched() call in drivers/cpuidle/cpuidle.c returns true, > how do we restart the timer? If the call returns false, we can restart the > timer in the cpuidle enter callback. Hi Amar, I think you should not use cpuidle prepare callback at all. It may be removed soon (see https://lkml.org/lkml/2011/6/6/261) and I think there are better ways to achieve what you are trying to do. I think everything should go into the enter routines (the idle routines provided by the driver). That way you would not have to worry about need_resched() in cpuidle.c. Also it would be a cleaner implementation as you wouldn't touch generic cpuidle code. > > The solution to the problem that we have in mind is adding an unprepare > callback to the cpuidle_device struct, and calling it if needs_resched() > returns true. Another option is to implement deferred timers for hrtimers. > Which of the two options is the better solution, or is there another > feasible alternative? As i said, everything should go inside enter routine and you wouldn't have to use/implement prepare/unprepare callbacks. Thanks -Trinabh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-07 6:25 ` Trinabh Gupta @ 2011-07-07 19:52 ` asinghal 2011-07-08 13:03 ` Deepthi Dharwar 0 siblings, 1 reply; 8+ messages in thread From: asinghal @ 2011-07-07 19:52 UTC (permalink / raw) To: Trinabh Gupta; +Cc: linux-pm, asinghal, johlstei 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. So cancelling the hrtimer that would affect the residency value calculated in the menu governor, in the enter callback is not possible. The timer needs to be cancelled before the select call is made. thanks, amar > On 07/06/2011 04:23 PM, asinghal@codeaurora.org wrote: >> We plan to use high resolution timers in one of our modules, with the >> requirement that we cancel these timers when the cpu goes idle and >> restart >> them when the cpu comes out of idle. >> >> We are cancelling the timers in cpuidle prepare callback. The problem is >> that if the need_resched() call in drivers/cpuidle/cpuidle.c returns >> true, >> how do we restart the timer? If the call returns false, we can restart >> the >> timer in the cpuidle enter callback. > > Hi Amar, > > I think you should not use cpuidle prepare callback at all. It may be > removed soon (see https://lkml.org/lkml/2011/6/6/261) and I think > there are better ways to achieve what you are trying to do. > > I think everything should go into the enter routines (the idle routines > provided by the driver). That way you would not have to worry about > need_resched() in cpuidle.c. Also it would be a cleaner implementation > as you wouldn't touch generic cpuidle code. > >> >> The solution to the problem that we have in mind is adding an unprepare >> callback to the cpuidle_device struct, and calling it if needs_resched() >> returns true. Another option is to implement deferred timers for >> hrtimers. >> Which of the two options is the better solution, or is there another >> feasible alternative? > > As i said, everything should go inside enter routine and > you wouldn't have to use/implement prepare/unprepare callbacks. > > Thanks > -Trinabh > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-07 19:52 ` asinghal @ 2011-07-08 13:03 ` Deepthi Dharwar 2011-07-11 17:26 ` asinghal 0 siblings, 1 reply; 8+ messages in thread From: Deepthi Dharwar @ 2011-07-08 13:03 UTC (permalink / raw) To: asinghal; +Cc: linux-pm, johlstei 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 ? > So cancelling the hrtimer that would affect the residency value calculated > in the menu governor, in the enter callback is not possible. The timer > needs to be cancelled before the select call is made. > > thanks, > amar > > > >> On 07/06/2011 04:23 PM, asinghal@codeaurora.org wrote: >>> We plan to use high resolution timers in one of our modules, with the >>> requirement that we cancel these timers when the cpu goes idle and >>> restart >>> them when the cpu comes out of idle. >>> >>> We are cancelling the timers in cpuidle prepare callback. The problem is >>> that if the need_resched() call in drivers/cpuidle/cpuidle.c returns >>> true, >>> how do we restart the timer? If the call returns false, we can restart >>> the >>> timer in the cpuidle enter callback. >> >> Hi Amar, >> >> I think you should not use cpuidle prepare callback at all. It may be >> removed soon (see https://lkml.org/lkml/2011/6/6/261) and I think >> there are better ways to achieve what you are trying to do. >> >> I think everything should go into the enter routines (the idle routines >> provided by the driver). That way you would not have to worry about >> need_resched() in cpuidle.c. Also it would be a cleaner implementation >> as you wouldn't touch generic cpuidle code. >> >>> >>> The solution to the problem that we have in mind is adding an unprepare >>> callback to the cpuidle_device struct, and calling it if needs_resched() >>> returns true. Another option is to implement deferred timers for >>> hrtimers. >>> Which of the two options is the better solution, or is there another >>> feasible alternative? >> >> As i said, everything should go inside enter routine and >> you wouldn't have to use/implement prepare/unprepare callbacks. >> >> Thanks >> -Trinabh >> > > > _______________________________________________ > linux-pm mailing list > linux-pm@lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/linux-pm Regards, Deepthi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-08 13:03 ` Deepthi Dharwar @ 2011-07-11 17:26 ` asinghal 2011-07-11 19:08 ` Vaidyanathan Srinivasan 0 siblings, 1 reply; 8+ messages in thread From: asinghal @ 2011-07-11 17:26 UTC (permalink / raw) To: Deepthi Dharwar; +Cc: linux-pm, asinghal, johlstei 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 function tick_nohz_stop_sched_tick.( so shld the prepare callback be moved before that in cpu_idle ??) 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. 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. thanks, amar >> So cancelling the hrtimer that would affect the residency value >> calculated >> in the menu governor, in the enter callback is not possible. The timer >> needs to be cancelled before the select call is made. >> >> thanks, >> amar >> >> >> >>> On 07/06/2011 04:23 PM, asinghal@codeaurora.org wrote: >>>> We plan to use high resolution timers in one of our modules, with the >>>> requirement that we cancel these timers when the cpu goes idle and >>>> restart >>>> them when the cpu comes out of idle. >>>> >>>> We are cancelling the timers in cpuidle prepare callback. The problem >>>> is >>>> that if the need_resched() call in drivers/cpuidle/cpuidle.c returns >>>> true, >>>> how do we restart the timer? If the call returns false, we can restart >>>> the >>>> timer in the cpuidle enter callback. >>> >>> Hi Amar, >>> >>> I think you should not use cpuidle prepare callback at all. It may be >>> removed soon (see https://lkml.org/lkml/2011/6/6/261) and I think >>> there are better ways to achieve what you are trying to do. >>> >>> I think everything should go into the enter routines (the idle routines >>> provided by the driver). That way you would not have to worry about >>> need_resched() in cpuidle.c. Also it would be a cleaner implementation >>> as you wouldn't touch generic cpuidle code. >>> >>>> >>>> The solution to the problem that we have in mind is adding an >>>> unprepare >>>> callback to the cpuidle_device struct, and calling it if >>>> needs_resched() >>>> returns true. Another option is to implement deferred timers for >>>> hrtimers. >>>> Which of the two options is the better solution, or is there another >>>> feasible alternative? >>> >>> As i said, everything should go inside enter routine and >>> you wouldn't have to use/implement prepare/unprepare callbacks. >>> >>> Thanks >>> -Trinabh >>> >> >> >> _______________________________________________ >> linux-pm mailing list >> linux-pm@lists.linux-foundation.org >> https://lists.linux-foundation.org/mailman/listinfo/linux-pm > Regards, > Deepthi > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-11 17:26 ` asinghal @ 2011-07-11 19:08 ` Vaidyanathan Srinivasan 2011-07-11 21:45 ` asinghal 0 siblings, 1 reply; 8+ messages in thread From: Vaidyanathan Srinivasan @ 2011-07-11 19:08 UTC (permalink / raw) To: asinghal; +Cc: linux-pm, johlstei * asinghal@codeaurora.org <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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-11 19:08 ` Vaidyanathan Srinivasan @ 2011-07-11 21:45 ` asinghal 2011-07-12 18:32 ` Vaidyanathan Srinivasan 0 siblings, 1 reply; 8+ messages in thread From: asinghal @ 2011-07-11 21:45 UTC (permalink / raw) To: svaidy; +Cc: linux-pm, asinghal, johlstei hello Vaidy, doesn't tick_nohz_get_sleep_length just return ts->sleep_length variable value from the tick_sched structure for that cpu ? The calculation of this sleep_length variable happens in function tick_nohz_stop_Sched_tick after call to function get_next_timer_interrupt ?? thanks, amar > * asinghal@codeaurora.org <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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] Unprepare callback for cpuidle_device 2011-07-11 21:45 ` asinghal @ 2011-07-12 18:32 ` Vaidyanathan Srinivasan 0 siblings, 0 replies; 8+ messages in thread From: Vaidyanathan Srinivasan @ 2011-07-12 18:32 UTC (permalink / raw) To: asinghal; +Cc: linux-pm, johlstei * asinghal@codeaurora.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 <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 > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-07-12 18:32 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-06 20:23 [RFC] Unprepare callback for cpuidle_device asinghal 2011-07-07 6:25 ` Trinabh Gupta 2011-07-07 19:52 ` asinghal 2011-07-08 13:03 ` Deepthi Dharwar 2011-07-11 17:26 ` asinghal 2011-07-11 19:08 ` Vaidyanathan Srinivasan 2011-07-11 21:45 ` asinghal 2011-07-12 18:32 ` Vaidyanathan Srinivasan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.