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.
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.
Signed-off-by: Colin Cross <[email protected]>
---
drivers/cpuidle/coupled.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index 3e65de1..b02810a 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -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:
--
1.7.7.3
cpuidle_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.
Signed-off-by: Colin Cross <[email protected]>
---
drivers/cpuidle/coupled.c | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index b02810a..fc427fa 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -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:
/*
--
1.7.7.3
On Friday 18 May 2012 11:35 PM, Colin Cross wrote:
> 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
On 05/18/2012 02:05 PM, Colin Cross wrote:
> 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.
> Or I can reupload the
> whole stack as v5 if you prefer.
no need.
thanks,
-Len Brown, Intel Open Source Technology Center
Hi Len,
Len Brown <[email protected]> writes:
> On 05/18/2012 02:05 PM, Colin Cross wrote:
>
>> 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.
>
>> 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