[PATCH] cpufreq: Schedule work for the first-online CPU on resume

[PATCH] cpufreq: Schedule work for the first-online CPU on resume

[Viresh Kumar]

All CPUs leaving the first-online CPU are hotplugged out on suspend and
and cpufreq core stops managing them.

On resume, we need to call cpufreq_update_policy() for this CPU's policy
to make sure its frequency is in sync with cpufreq's cached value, as it
might have got updated by hardware during suspend/resume.

The policies are always added to the top of the policy-list. So, in
normal circumstances, CPU 0's policy will be the last one in the list.
And so the code checks for the last policy.

But there are cases where it will fail. Consider quad-core system, with
policy-per core. If CPU0 is hotplugged out and added back again, the
last policy will be on CPU1 :(

To fix this in a proper way, always look for the policy of the first
online CPU. That way we will be sure that we are calling
cpufreq_update_policy() for the only CPU that wasn't hotplugged out.

Cc: 3.15+ <stable@vger.kernel.org> # 3.15+
Fixes: 2f0aea936360 ("cpufreq: suspend governors on system suspend/hibernate")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 28e59a48b35f..8ae655c364f4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1698,15 +1698,18 @@ void cpufreq_resume(void)
 		    || __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))
 			pr_err("%s: Failed to start governor for policy: %p\n",
 				__func__, policy);
-
-		/*
-		 * schedule call cpufreq_update_policy() for boot CPU, i.e. last
-		 * policy in list. It will verify that the current freq is in
-		 * sync with what we believe it to be.
-		 */
-		if (list_is_last(&policy->policy_list, &cpufreq_policy_list))
-			schedule_work(&policy->update);
 	}
+
+	/*
+	 * schedule call cpufreq_update_policy() for first-online CPU, as that
+	 * wouldn't be hotplugged-out on suspend. It will verify that the
+	 * current freq is in sync with what we believe it to be.
+	 */
+	policy = cpufreq_cpu_get_raw(cpumask_first(cpu_online_mask));
+	if (WARN_ON(!policy))
+		return;
+
+	schedule_work(&policy->update);
 }
 
 /**
-- 
2.3.0.rc0.44.ga94655d

Re: [PATCH] cpufreq: Schedule work for the first-online CPU on resume

[Saravana Kannan]

On 04/01/2015 09:51 PM, Viresh Kumar wrote:
> All CPUs leaving the first-online CPU are hotplugged out on suspend and
> and cpufreq core stops managing them.
>
> On resume, we need to call cpufreq_update_policy() for this CPU's policy
> to make sure its frequency is in sync with cpufreq's cached value, as it
> might have got updated by hardware during suspend/resume.
>
> The policies are always added to the top of the policy-list. So, in
> normal circumstances, CPU 0's policy will be the last one in the list.
> And so the code checks for the last policy.
>
> But there are cases where it will fail. Consider quad-core system, with
> policy-per core. If CPU0 is hotplugged out and added back again, the
> last policy will be on CPU1 :(
>
> To fix this in a proper way, always look for the policy of the first
> online CPU. That way we will be sure that we are calling
> cpufreq_update_policy() for the only CPU that wasn't hotplugged out.
>
> Cc: 3.15+ <stable@vger.kernel.org> # 3.15+
> Fixes: 2f0aea936360 ("cpufreq: suspend governors on system suspend/hibernate")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

"Reported by" or something similar for me identifying the issue?

> ---
>   drivers/cpufreq/cpufreq.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 28e59a48b35f..8ae655c364f4 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1698,15 +1698,18 @@ void cpufreq_resume(void)
>   		    || __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))
>   			pr_err("%s: Failed to start governor for policy: %p\n",
>   				__func__, policy);
> -
> -		/*
> -		 * schedule call cpufreq_update_policy() for boot CPU, i.e. last
> -		 * policy in list. It will verify that the current freq is in
> -		 * sync with what we believe it to be.
> -		 */
> -		if (list_is_last(&policy->policy_list, &cpufreq_policy_list))
> -			schedule_work(&policy->update);
>   	}
> +
> +	/*
> +	 * schedule call cpufreq_update_policy() for first-online CPU, as that
> +	 * wouldn't be hotplugged-out on suspend. It will verify that the
> +	 * current freq is in sync with what we believe it to be.
> +	 */
> +	policy = cpufreq_cpu_get_raw(cpumask_first(cpu_online_mask));
> +	if (WARN_ON(!policy))
> +		return;
> +
> +	schedule_work(&policy->update);
>   }
>
>   /**
>

Acked-by: Saravana Kannan <skannan@codeaurora.org>


-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] cpufreq: Schedule work for the first-online CPU on resume

[Viresh Kumar]

On 3 April 2015 at 00:08, Saravana Kannan <skannan@codeaurora.org> wrote:

> "Reported by" or something similar for me identifying the issue?

Oh yes, forgot :( ..

You could have added that too :)

Reported-by: Saravana Kannan <skannan@codeaurora.org>

> Acked-by: Saravana Kannan <skannan@codeaurora.org>