thermal: cpu_cooling: Power calculation when CPUs are hotplugged

thermal: cpu_cooling: Power calculation when CPUs are hotplugged

[Kapileshwar Singh]

The following patch series aims at correcting the requested power
calculation when CPUs are hotplugged out. These patches are based on
the "linus" branch of Eduardo's tree.

There is no need to update the cpu_dev pointer (cached) on the reception of a
policy CPU update notifier from cpufreq. This is not required as OPPs
can be ascertained even if the CPU is offline.

cpufreq_get_requested power ensures that a CPU is online before calculating
the frequency for the CPUs and returns the power as 0 if all the CPUs
are offline in the cooling device.

Regards,
KP

[PATCH 1/2] thermal: cpu_cooling: Remove cpu_dev update on policy CPU update

[Kapileshwar Singh]

It was initially understood that an update to the cpu_device
(cached in cpufreq_cooling_device) was required to ascertain the
correct operating point of the device on a cpufreq policy->cpu update
or creation or deletion of a cpufreq policy.
(e.g. when the existing policy CPU goes offline).

This was added in:

e0128d8ab423 - thermal: cpu_cooling: implement the power cooling device API

This update is not required and it is possible to ascertain the OPPs
from the leading CPU in a cpufreq domain even if the CPU is hotplugged out.

Acked-by: Javi Merino <javi.merino@arm.com>
Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---
 drivers/thermal/cpu_cooling.c |   40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index c4974144c787..2e15133b4793 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -199,39 +199,6 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
 }
 EXPORT_SYMBOL_GPL(cpufreq_cooling_get_level);
 
-static void update_cpu_device(int cpu)
-{
-	struct cpufreq_cooling_device *cpufreq_dev;
-
-	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
-		if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) {
-			cpufreq_dev->cpu_dev = get_cpu_device(cpu);
-			if (!cpufreq_dev->cpu_dev) {
-				dev_warn(&cpufreq_dev->cool_dev->device,
-					"No cpu device for new policy cpu %d\n",
-					 cpu);
-			}
-			break;
-		}
-	}
-	mutex_unlock(&cooling_cpufreq_lock);
-}
-
-static void remove_cpu_device(int cpu)
-{
-	struct cpufreq_cooling_device *cpufreq_dev;
-
-	mutex_lock(&cooling_cpufreq_lock);
-	list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
-		if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) {
-			cpufreq_dev->cpu_dev = NULL;
-			break;
-		}
-	}
-	mutex_unlock(&cooling_cpufreq_lock);
-}
-
 /**
  * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
  * @nb:	struct notifier_block * with callback info.
@@ -268,13 +235,6 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
 		}
 		mutex_unlock(&cooling_cpufreq_lock);
 		break;
-
-	case CPUFREQ_CREATE_POLICY:
-		update_cpu_device(policy->cpu);
-		break;
-	case CPUFREQ_REMOVE_POLICY:
-		remove_cpu_device(policy->cpu);
-		break;
 	default:
 		return NOTIFY_DONE;
 	}
-- 
1.7.9.5

[PATCH 2/2] thermal: cpu_cooling: Fix power calculation when CPUs are offline

[Kapileshwar Singh]

Ensure that the CPU for which the frequency is being requested
is online. If none of the CPUs are online the requested power is
returned as 0.

Acked-by: Javi Merino <javi.merino@arm.com>
Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---
 drivers/thermal/cpu_cooling.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 2e15133b4793..07a9629edf4b 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -555,7 +555,18 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 	struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
 	u32 *load_cpu = NULL;
 
-	freq = cpufreq_quick_get(cpumask_any(&cpufreq_device->allowed_cpus));
+	cpu = cpumask_any_and(&cpufreq_device->allowed_cpus, cpu_online_mask);
+
+	/*
+	 * All the CPUs are offline, thus the requested power by
+	 * the cdev is 0
+	 */
+	if (cpu >= nr_cpu_ids) {
+		*power = 0;
+		return 0;
+	}
+
+	freq = cpufreq_quick_get(cpu);
 
 	if (trace_thermal_power_cpu_get_power_enabled()) {
 		u32 ncpus = cpumask_weight(&cpufreq_device->allowed_cpus);
-- 
1.7.9.5

Re: [PATCH 1/2] thermal: cpu_cooling: Remove cpu_dev update on policy CPU update

[Eduardo Valentin]

KP,

On Wed, Mar 11, 2015 at 05:53:36PM +0000, Kapileshwar Singh wrote:
> It was initially understood that an update to the cpu_device
> (cached in cpufreq_cooling_device) was required to ascertain the
> correct operating point of the device on a cpufreq policy->cpu update
> or creation or deletion of a cpufreq policy.
> (e.g. when the existing policy CPU goes offline).
> 
> This was added in:
> 
> e0128d8ab423 - thermal: cpu_cooling: implement the power cooling device API


Can you please add a 'fixes' entry here?

> 
> This update is not required and it is possible to ascertain the OPPs
> from the leading CPU in a cpufreq domain even if the CPU is hotplugged out.
> 
> Acked-by: Javi Merino <javi.merino@arm.com>
> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> ---
>  drivers/thermal/cpu_cooling.c |   40 ----------------------------------------

Your patch looks fine to me, but you have to resend it (same applies to
patch 02). The reason is that I've got it with the charset issue and
characters are mangled. Can you please check with Javi or Punit on how
to setup your mailer?

Thanks,

Eduardo Valentin
>  1 file changed, 40 deletions(-)
> 
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index c4974144c787..2e15133b4793 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -199,39 +199,6 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_cooling_get_level);
>  
> -static void update_cpu_device(int cpu)
> -{
> -	struct cpufreq_cooling_device *cpufreq_dev;
> -
> -	mutex_lock(&cooling_cpufreq_lock);
> -	list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
> -		if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) {
> -			cpufreq_dev->cpu_dev = get_cpu_device(cpu);
> -			if (!cpufreq_dev->cpu_dev) {
> -				dev_warn(&cpufreq_dev->cool_dev->device,
> -					"No cpu device for new policy cpu %d\n",
> -					 cpu);
> -			}
> -			break;
> -		}
> -	}
> -	mutex_unlock(&cooling_cpufreq_lock);
> -}
> -
> -static void remove_cpu_device(int cpu)
> -{
> -	struct cpufreq_cooling_device *cpufreq_dev;
> -
> -	mutex_lock(&cooling_cpufreq_lock);
> -	list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
> -		if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) {
> -			cpufreq_dev->cpu_dev = NULL;
> -			break;
> -		}
> -	}
> -	mutex_unlock(&cooling_cpufreq_lock);
> -}
> -
>  /**
>   * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
>   * @nb:	struct notifier_block * with callback info.
> @@ -268,13 +235,6 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
>  		}
>  		mutex_unlock(&cooling_cpufreq_lock);
>  		break;
> -
> -	case CPUFREQ_CREATE_POLICY:
> -		update_cpu_device(policy->cpu);
> -		break;
> -	case CPUFREQ_REMOVE_POLICY:
> -		remove_cpu_device(policy->cpu);
> -		break;
>  	default:
>  		return NOTIFY_DONE;
>  	}
> -- 
> 1.7.9.5
> 
>