bug fixes for coupled cpuidle
cover bug fixes for coupled cpuidle
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3). Or I can reupload the whole stack as v5 if you prefer.
PATCH 1/2 cpuidle: coupled: fix count of online cpus
online_count was never incremented on boot, and was also counting cpus that were not part of the coupled set. Fix both issues by introducting a new function that counts online coupled cpus, and call it from register as well as the hotplug notifier.
--- drivers/cpuidle/coupled.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
drivers/cpuidle/coupled.c +10 -1
@@ -532,6 +532,13 @@ int cpuidle_enter_state_coupled(struct cpuidle_device *dev,return entered_state; } +static void cpuidle_coupled_update_online_cpus (struct cpuidle_coupled *coupled) +{ +cpumask_t cpus; +cpumask_and (&cpus, cpu_online_mask, &coupled->coupled_cpus); + coupled->online_count =cpumask_weight (&cpus); +} +/** * cpuidle_coupled_register_device - register a coupled cpuidle device * @dev: struct cpuidle_device for the current cpu
@@ -570,6 +577,8 @@ int cpuidle_coupled_register_device(struct cpuidle_device *dev)if (WARN_ON (!cpumask_equal (&dev->coupled_cpus, &coupled->coupled_cpus))) coupled->prevent++; +cpuidle_coupled_update_online_cpus (coupled); + coupled->refcnt++; csd = &per_cpu (cpuidle_coupled_poke_cb, dev->cpu);
@@ -668,7 +677,7 @@ static int cpuidle_coupled_cpu_notify(struct notifier_block *nb,break ;case CPU_ONLINE :case CPU_DEAD : - dev->coupled->online_count =num_online_cpus (); +cpuidle_coupled_update_online_cpus (dev->coupled);/* Fall through */ case CPU_UP_CANCELED :case CPU_DOWN_FAILED :
Signature · 1 line
1.7.7.3
PATCH 2/2 cpuidle: coupled: fix decrementing ready count
In reply to Colin Crosscpuidle_coupled_set_not_ready sometimes refuses to decrement the ready count in order to prevent a race condition. This makes it unsuitable for use when finished with idle. Add a new function cpuidle_coupled_set_done that decrements both the ready count and waiting count, and call it after idle is complete.
--- drivers/cpuidle/coupled.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-)
drivers/cpuidle/coupled.c +16 -2
@@ -388,6 +388,21 @@ static void cpuidle_coupled_set_not_waiting(int cpu, }/** +* cpuidle_coupled_set_done - mark this cpu as leaving the ready loop +* @cpu: the current cpu +* @coupled: the struct coupled that contains the current cpu +* +* Marks this cpu as no longer in the ready and waiting loops. Decrements +* the waiting count first to prevent another cpu looping back in and seeing +* this cpu as waiting just before it exits idle. +*/ +static void cpuidle_coupled_set_done (int cpu,struct cpuidle_coupled *coupled) +{ +cpuidle_coupled_set_not_waiting (cpu, coupled); +atomic_sub (MAX_WAITING_CPUS , &coupled->ready_waiting_counts); +} + +/** * cpuidle_coupled_clear_pokes - spin until the poke interrupt is processed * @cpu - this cpu *
@@ -501,8 +516,7 @@ int cpuidle_enter_state_coupled(struct cpuidle_device *dev, entered_state =cpuidle_enter_state (dev, drv, next_state); -cpuidle_coupled_set_not_waiting (dev->cpu, coupled); -cpuidle_coupled_set_not_ready (coupled); +cpuidle_coupled_set_done (dev->cpu, coupled); out:/*
Signature · 1 line
1.7.7.3
Quoted 10 lines · Colin Cross
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3). Or I can reupload the whole stack as v5 if you prefer.
I confirm that these two fixes are needed to get couple idle v4 series working.
Regards
Santosh
cover bug fixes for coupled cpuidle
Len,
Quoted 7 lines · Santosh Shilimkar
Quoted 6 lines · Colin Cross
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Quoted 7 lines
Quoted 4 lines
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3). Or I can reupload the whole stack as v5 if you prefer.
I confirm that these two fixes are needed to get couple idle v4 series working.
Can you pick these up for v3.6?
I don't currently see them in your next branch.
Thanks,
Kevin
Quoted 9 lines · Colin Cross
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3).
squash & tags update done.
Quoted 2 lines
Or I can reupload the
whole stack as v5 if you prefer.
no need.
thanks,
-Len Brown, Intel Open Source Technology Center
Quoted 16 lines · Len Brown
Quoted 9 lines · Colin Cross
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3).
squash & tags update done.
Quoted 8 lines
Quoted 2 lines
Or I can reupload the
whole stack as v5 if you prefer.no need.
thanks,
-Len Brown, Intel Open Source Technology Center
Thanks!
Hi Len,
Quoted 13 lines · Len Brown
Quoted 9 lines · Colin Cross
The last modifications made to the coupled cpuidle patches introduced two bugs that I missed during testing. The online count was never initialized, causing coupled idle to always wait and never enter the ready loop. That hid the second bug, the ready count could never be decremented after exiting idle.
Len, these two patches could be squashed into patch 3 of the original set. If you do squash them, you could also add Rafael's tags to the set (Reviewed-by on 1 and 2, acked-by on 3).
Quoted 7 lines
squash & tags update done.
Quoted 2 lines
Or I can reupload the
whole stack as v5 if you prefer.no need.
Hmm, after the problems with the pull request, it looks like you dropped the coupled CPUidle series completely. It's a shame that this has been reviewed, tested and queued for so long only to see it dropped at the last minute.
Instead of the squash, could you just queue the original series (that has been in linux-next for some time) and then submit the fixes in $SUBJECT series for v3.5-rc? That way we could still get this support in for 3.5, which many of us are waiting for.
Thanks,
Kevin