Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- kernel/sched/idle.c | 161 +++++++++++++++++++++++++++------------------------ 1 file changed, 86 insertions(+), 75 deletions(-)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index b8cd302..f2f4bc9 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -63,6 +63,90 @@ void __weak arch_cpu_idle(void) local_irq_enable(); }
+#ifdef CONFIG_CPU_IDLE +static int __cpuidle_idle_call(void) +{ + struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices); + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); + int next_state, entered_state, ret; + bool broadcast; + + /* + * Check if the cpuidle framework is ready, otherwise fallback + * to the default arch specific idle method + */ + ret = cpuidle_enabled(drv, dev); + if (ret) + return ret; + + /* + * Ask the governor to choose an idle state it thinks + * it is convenient to go to. There is *always* a + * convenient idle state + */ + next_state = cpuidle_select(drv, dev); + + /* + * The idle task must be scheduled, it is pointless to + * go to idle, just update no idle residency and get + * out of this function + */ + if (current_clr_polling_and_test()) { + dev->last_residency = 0; + entered_state = next_state; + local_irq_enable(); + } else { + broadcast = !!(drv->states[next_state].flags & + CPUIDLE_FLAG_TIMER_STOP); + + if (broadcast) + /* + * Tell the time framework to switch to a + * broadcast timer because our local timer + * will be shutdown. If a local timer is used + * from another cpu as a broadcast timer, this + * call may fail if it is not available + */ + ret = clockevents_notify( + CLOCK_EVT_NOTIFY_BROADCAST_ENTER, + &dev->cpu); + + if (!ret) { + trace_cpu_idle_rcuidle(next_state, dev->cpu); + + /* + * Enter the idle state previously returned by + * the governor decision. This function will + * block until an interrupt occurs and will + * take care of re-enabling the local + * interrupts + */ + entered_state = cpuidle_enter(drv, dev, next_state); + + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu); + + if (broadcast) + clockevents_notify( + CLOCK_EVT_NOTIFY_BROADCAST_EXIT, + &dev->cpu); + + /* + * Give the governor an opportunity to reflect + * on the outcome + */ + cpuidle_reflect(dev, entered_state); + } + } + + return 0; +} +#else +static int inline __cpuidle_idle_call(void) +{ + return -ENOSYS; +} +#endif + /** * cpuidle_idle_call - the main idle function * @@ -70,10 +154,7 @@ void __weak arch_cpu_idle(void) */ static void cpuidle_idle_call(void) { - struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices); - struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); - int next_state, entered_state, ret; - bool broadcast; + int ret;
/* * Check if the idle task must be rescheduled. If it is the @@ -100,80 +181,10 @@ static void cpuidle_idle_call(void) rcu_idle_enter();
/* - * Check if the cpuidle framework is ready, otherwise fallback - * to the default arch specific idle method - */ - ret = cpuidle_enabled(drv, dev); - - if (!ret) { - /* - * Ask the governor to choose an idle state it thinks - * it is convenient to go to. There is *always* a - * convenient idle state - */ - next_state = cpuidle_select(drv, dev); - - /* - * The idle task must be scheduled, it is pointless to - * go to idle, just update no idle residency and get - * out of this function - */ - if (current_clr_polling_and_test()) { - dev->last_residency = 0; - entered_state = next_state; - local_irq_enable(); - } else { - broadcast = !!(drv->states[next_state].flags & - CPUIDLE_FLAG_TIMER_STOP); - - if (broadcast) - /* - * Tell the time framework to switch - * to a broadcast timer because our - * local timer will be shutdown. If a - * local timer is used from another - * cpu as a broadcast timer, this call - * may fail if it is not available - */ - ret = clockevents_notify( - CLOCK_EVT_NOTIFY_BROADCAST_ENTER, - &dev->cpu); - - if (!ret) { - trace_cpu_idle_rcuidle(next_state, dev->cpu); - - /* - * Enter the idle state previously - * returned by the governor - * decision. This function will block - * until an interrupt occurs and will - * take care of re-enabling the local - * interrupts - */ - entered_state = cpuidle_enter(drv, dev, - next_state); - - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, - dev->cpu); - - if (broadcast) - clockevents_notify( - CLOCK_EVT_NOTIFY_BROADCAST_EXIT, - &dev->cpu); - - /* - * Give the governor an opportunity to reflect on the - * outcome - */ - cpuidle_reflect(dev, entered_state); - } - } - } - - /* * We can't use the cpuidle framework, let's use the default * idle routine */ + ret = __cpuidle_idle_call(); if (ret) arch_cpu_idle();
When the cpu enters idle it stores the cpuidle state pointer in the struct rq which in turn could be used to take a right decision when balancing a task.
As soon as the cpu exits the idle state, the structure is filled back with the NULL pointer.
There are a couple of situations where the idle state pointer could be changed while looking at it:
1. For x86/acpi with dynamic c-states, when a laptop switches from battery to AC that could result on removing the deeper idle state. The acpi driver triggers: 'acpi_processor_cst_has_changed' 'cpuidle_pause_and_lock' 'cpuidle_uninstall_idle_handler' 'kick_all_cpus_sync'.
All cpus will exit their idle state and the pointed object will be set to NULL.
2. The cpuidle driver is unloaded. Logically that could happen but not in practice because the drivers are always compiled in and 95% of the drivers are not coded to unregister the driver. In any case, the unloading code must call 'cpuidle_unregister_device', that calls 'cpuidle_pause_and_lock' leading to 'kick_all_cpus_sync' as mentioned above.
The race can happen if we use the pointer and then one of these two situations occurs at the same moment.
In order to be safe, the idle state pointer stored in the rq must be used inside a rcu_read_lock section where we are protected with the 'rcu_barrier' in the 'cpuidle_uninstall_idle_handler' function.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org --- drivers/cpuidle/cpuidle.c | 6 ++++++ kernel/sched/idle.c | 7 +++++++ kernel/sched/sched.h | 5 +++++ 3 files changed, 18 insertions(+)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 8236746..6a13f40 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -190,6 +190,12 @@ void cpuidle_install_idle_handler(void) */ void cpuidle_uninstall_idle_handler(void) { + /* + * Wait for the scheduler to finish to use the idle state he + * may pointing to when looking for the cpu idle states + */ + rcu_barrier(); + if (enabled_devices) { initialized = 0; kick_all_cpus_sync(); diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index f2f4bc9..09029f6 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -12,6 +12,8 @@
#include <trace/events/power.h>
+#include "sched.h" + static int __read_mostly cpu_idle_force_poll;
void cpu_idle_poll_ctrl(bool enable) @@ -66,6 +68,7 @@ void __weak arch_cpu_idle(void) #ifdef CONFIG_CPU_IDLE static int __cpuidle_idle_call(void) { + struct rq *rq = this_rq(); struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices); struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); int next_state, entered_state, ret; @@ -114,6 +117,8 @@ static int __cpuidle_idle_call(void) if (!ret) { trace_cpu_idle_rcuidle(next_state, dev->cpu);
+ rq->idle_state = &drv->states[next_state]; + /* * Enter the idle state previously returned by * the governor decision. This function will @@ -123,6 +128,8 @@ static int __cpuidle_idle_call(void) */ entered_state = cpuidle_enter(drv, dev, next_state);
+ rq->idle_state = NULL; + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
if (broadcast) diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 456e492..bace64a 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -14,6 +14,7 @@ #include "cpuacct.h"
struct rq; +struct cpuidle_state;
extern __read_mostly int scheduler_running;
@@ -643,6 +644,10 @@ struct rq { #ifdef CONFIG_SMP struct llist_head wake_list; #endif + +#ifdef CONFIG_CPU_IDLE + struct cpuidle_state *idle_state; +#endif };
static inline int cpu_of(struct rq *rq)
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Any ideas how to resolve that?
kernel/sched/idle.c | 161 +++++++++++++++++++++++++++------------------------ 1 file changed, 86 insertions(+), 75 deletions(-)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index b8cd302..f2f4bc9 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -63,6 +63,90 @@ void __weak arch_cpu_idle(void) local_irq_enable(); } +#ifdef CONFIG_CPU_IDLE +static int __cpuidle_idle_call(void) +{
- struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
- struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
- int next_state, entered_state, ret;
- bool broadcast;
- /*
* Check if the cpuidle framework is ready, otherwise fallback* to the default arch specific idle method*/- ret = cpuidle_enabled(drv, dev);
- if (ret)
return ret;- /*
* Ask the governor to choose an idle state it thinks* it is convenient to go to. There is *always* a* convenient idle state*/- next_state = cpuidle_select(drv, dev);
- /*
* The idle task must be scheduled, it is pointless to* go to idle, just update no idle residency and get* out of this function*/- if (current_clr_polling_and_test()) {
dev->last_residency = 0;entered_state = next_state;local_irq_enable();- } else {
broadcast = !!(drv->states[next_state].flags &CPUIDLE_FLAG_TIMER_STOP);if (broadcast)/** Tell the time framework to switch to a* broadcast timer because our local timer* will be shutdown. If a local timer is used* from another cpu as a broadcast timer, this* call may fail if it is not available*/ret = clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER,&dev->cpu);if (!ret) {trace_cpu_idle_rcuidle(next_state, dev->cpu);/** Enter the idle state previously returned by* the governor decision. This function will* block until an interrupt occurs and will* take care of re-enabling the local* interrupts*/entered_state = cpuidle_enter(drv, dev, next_state);trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);if (broadcast)clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT,&dev->cpu);/** Give the governor an opportunity to reflect* on the outcome*/cpuidle_reflect(dev, entered_state);}- }
- return 0;
+} +#else +static int inline __cpuidle_idle_call(void) +{
- return -ENOSYS;
+} +#endif
/**
- cpuidle_idle_call - the main idle function
@@ -70,10 +154,7 @@ void __weak arch_cpu_idle(void) */ static void cpuidle_idle_call(void) {
- struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
- struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
- int next_state, entered_state, ret;
- bool broadcast;
- int ret;
/* * Check if the idle task must be rescheduled. If it is the @@ -100,80 +181,10 @@ static void cpuidle_idle_call(void) rcu_idle_enter(); /*
* Check if the cpuidle framework is ready, otherwise fallback* to the default arch specific idle method*/- ret = cpuidle_enabled(drv, dev);
- if (!ret) {
/** Ask the governor to choose an idle state it thinks* it is convenient to go to. There is *always* a* convenient idle state*/next_state = cpuidle_select(drv, dev);/** The idle task must be scheduled, it is pointless to* go to idle, just update no idle residency and get* out of this function*/if (current_clr_polling_and_test()) {dev->last_residency = 0;entered_state = next_state;local_irq_enable();} else {broadcast = !!(drv->states[next_state].flags &CPUIDLE_FLAG_TIMER_STOP);if (broadcast)/** Tell the time framework to switch* to a broadcast timer because our* local timer will be shutdown. If a* local timer is used from another* cpu as a broadcast timer, this call* may fail if it is not available*/ret = clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER,&dev->cpu);if (!ret) {trace_cpu_idle_rcuidle(next_state, dev->cpu);/** Enter the idle state previously* returned by the governor* decision. This function will block* until an interrupt occurs and will* take care of re-enabling the local* interrupts*/entered_state = cpuidle_enter(drv, dev,next_state);trace_cpu_idle_rcuidle(PWR_EVENT_EXIT,dev->cpu);if (broadcast)clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT,&dev->cpu);/** Give the governor an opportunity to reflect on the* outcome*/cpuidle_reflect(dev, entered_state);}}- }
- /*
*/
- We can't use the cpuidle framework, let's use the default
- idle routine
- ret = __cpuidle_idle_call(); if (ret) arch_cpu_idle();
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/ we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select() for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler should optimize out the blocks that depend on it being non-negative.
Thanks!
On 05/01/2014 12:56 AM, Rafael J. Wysocki wrote:
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/ we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select() for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler should optimize out the blocks that depend on it being non-negative.
Thanks for the head up.
Actually that was to solve a compilation issue with the next patch when adding the cpuidle state in the struct rq.
When the option CPU_IDLE is not set, the code assinging the cpu idle state in the rq is still there while in the struct rq the field is compiled out with the ifdef macro. If I rely on the compiler optimization, the compilation error will happen.
On Friday, May 02, 2014 10:59:11 AM Daniel Lezcano wrote:
On 05/01/2014 12:56 AM, Rafael J. Wysocki wrote:
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/ we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select() for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler should optimize out the blocks that depend on it being non-negative.
Thanks for the head up.
Actually that was to solve a compilation issue with the next patch when adding the cpuidle state in the struct rq.
When the option CPU_IDLE is not set, the code assinging the cpu idle state in the rq is still there while in the struct rq the field is compiled out with the ifdef macro. If I rely on the compiler optimization, the compilation error will happen.
I see.
If you don't put the new idle_state field in struct_rq under the #ifdef, you won't need to worry about the build problem.
Alternatively, you can define
#ifdef CONFIG_CPU_IDLE static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) { rq->idle_state = state; } #else static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) {} #endif
and use rq_set_idle_state() to set that field.
On 05/02/2014 02:09 PM, Rafael J. Wysocki wrote:
On Friday, May 02, 2014 10:59:11 AM Daniel Lezcano wrote:
On 05/01/2014 12:56 AM, Rafael J. Wysocki wrote:
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/ we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select() for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler should optimize out the blocks that depend on it being non-negative.
Thanks for the head up.
Actually that was to solve a compilation issue with the next patch when adding the cpuidle state in the struct rq.
When the option CPU_IDLE is not set, the code assinging the cpu idle state in the rq is still there while in the struct rq the field is compiled out with the ifdef macro. If I rely on the compiler optimization, the compilation error will happen.
I see.
If you don't put the new idle_state field in struct_rq under the #ifdef, you won't need to worry about the build problem.
Alternatively, you can define
#ifdef CONFIG_CPU_IDLE static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) { rq->idle_state = state; } #else static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) {} #endif
and use rq_set_idle_state() to set that field.
Thanks, I will look at one or another solution.
On Fri, 2 May 2014, Rafael J. Wysocki wrote:
On Friday, May 02, 2014 10:59:11 AM Daniel Lezcano wrote:
On 05/01/2014 12:56 AM, Rafael J. Wysocki wrote:
On Thursday, May 01, 2014 12:47:25 AM Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Moreover (along the lines of Nico said) after https://patchwork.kernel.org/patch/4071541/ we actually don't need the #ifdef CONFIG_CPU_IDLE in your patch, because cpuidle_select() for CONFIG_CPU_IDLE unset is a static inline returning a negative number and the compiler should optimize out the blocks that depend on it being non-negative.
Thanks for the head up.
Actually that was to solve a compilation issue with the next patch when adding the cpuidle state in the struct rq.
When the option CPU_IDLE is not set, the code assinging the cpu idle state in the rq is still there while in the struct rq the field is compiled out with the ifdef macro. If I rely on the compiler optimization, the compilation error will happen.
I see.
If you don't put the new idle_state field in struct_rq under the #ifdef, you won't need to worry about the build problem.
Alternatively, you can define
#ifdef CONFIG_CPU_IDLE static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) { rq->idle_state = state; } #else static inline void rq_set_idle_state(struct rq *rq, struct cpuidle_state *state) {} #endif
and use rq_set_idle_state() to set that field.
Agreed. And a similar accessor for consumers to get this value in order to avoid #ifdef's there as well.
Nicolas
On 05/01/2014 12:47 AM, Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Any ideas how to resolve that?
I don't think we have a big conflict. If Peter takes your patches before than mines then I will refresh and resend them.
I am open to any other suggestion.
Thanks -- Daniel
On Friday, May 02, 2014 10:52:27 AM Daniel Lezcano wrote:
On 05/01/2014 12:47 AM, Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Any ideas how to resolve that?
I don't think we have a big conflict. If Peter takes your patches before than mines then I will refresh and resend them.
Actually, I was planning the merge them myself, because they are more cpuidle than the scheduler, but either way would be fine.
I am open to any other suggestion.
Please see the other message I've just sent. :-)
On 05/02/2014 02:09 PM, Rafael J. Wysocki wrote:
On Friday, May 02, 2014 10:52:27 AM Daniel Lezcano wrote:
On 05/01/2014 12:47 AM, Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Any ideas how to resolve that?
I don't think we have a big conflict. If Peter takes your patches before than mines then I will refresh and resend them.
Actually, I was planning the merge them myself, because they are more cpuidle than the scheduler, but either way would be fine.
Well I have some patches for the scheduler which will need these modifications. Is it possible to merge them throw a common branch to be shared between sched and pm ?
I am open to any other suggestion.
Please see the other message I've just sent. :-)
On Friday, May 02, 2014 03:35:23 PM Daniel Lezcano wrote:
On 05/02/2014 02:09 PM, Rafael J. Wysocki wrote:
On Friday, May 02, 2014 10:52:27 AM Daniel Lezcano wrote:
On 05/01/2014 12:47 AM, Rafael J. Wysocki wrote:
On Wednesday, April 30, 2014 02:01:02 PM Daniel Lezcano wrote:
Encapsulate the large portion of cpuidle_idle_call inside another function so when CONFIG_CPU_IDLE=n, the code will be compiled out. Also that is benefitial for the clarity of the code as it removes a nested indentation level.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
Well, this conflicts with
https://patchwork.kernel.org/patch/4071541/
which you haven't commented on and I still want cpuidle_select() to be able to return negative values because of
https://patchwork.kernel.org/patch/4089631/
(and I have one more patch on top of these two that requires this).
Any ideas how to resolve that?
I don't think we have a big conflict. If Peter takes your patches before than mines then I will refresh and resend them.
Actually, I was planning the merge them myself, because they are more cpuidle than the scheduler, but either way would be fine.
Well I have some patches for the scheduler which will need these modifications. Is it possible to merge them throw a common branch to be shared between sched and pm ?
That would be perfectly fine by me, but I'm not sure what Ingo and Peter think about that.
I can set up a branch with sched/idle/cpuidle changes.
On Fri, May 02, 2014 at 05:32:25PM +0200, Rafael J. Wysocki wrote:
That would be perfectly fine by me, but I'm not sure what Ingo and Peter think about that.
I can set up a branch with sched/idle/cpuidle changes.
Ingo typically likes things like that. I'm still a git Luddite, I'll learn someday :-)
linaro-kernel@lists.linaro.org
-
Daniel Lezcano -
Nicolas Pitre -
Peter Zijlstra -
Rafael J. Wysocki