Re: [PATCH v3 2/5] thermal: add trace events to the power allocator governor

[Viresh Kumar]

Hi Javi,

Sorry for picking up an old thread, but i had a question for you.

On Mon, Mar 2, 2015 at 10:47 PM, Javi Merino  wrote:
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c

> @@ -588,12 +590,20 @@ static int cpufreq_get_requested_power(struct 
> thermal_cooling_device *cdev,
>u32 *power)
>  {
> unsigned long freq;
> -   int cpu, ret;
> +   int i = 0, cpu, ret;
> u32 static_power, dynamic_power, total_load = 0;
> struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
> +   u32 *load_cpu = NULL;
>
> freq = cpufreq_quick_get(cpumask_any(_device->allowed_cpus));
>
> +   if (trace_thermal_power_cpu_get_power_enabled()) {

You allocated load_cpu if cpu_get_power trace is enabled.

> +   u32 ncpus = cpumask_weight(_device->allowed_cpus);
> +
> +   load_cpu = devm_kcalloc(>device, ncpus, 
> sizeof(*load_cpu),
> +   GFP_KERNEL);
> +   }
> +
> for_each_cpu(cpu, _device->allowed_cpus) {
> u32 load;
>
> @@ -603,14 +613,29 @@ static int cpufreq_get_requested_power(struct 
> thermal_cooling_device *cdev,
> load = 0;
>
> total_load += load;
> +   if (trace_thermal_power_cpu_limit_enabled() && load_cpu)

But you store values to it only if cpu_limit trace is also enabled,
otherwise it is all zeros.

Why?

Re: [PATCH v3 2/5] thermal: add trace events to the power allocator governor

[Viresh Kumar]

Hi Javi,

Sorry for picking up an old thread, but i had a question for you.

On Mon, Mar 2, 2015 at 10:47 PM, Javi Merino  wrote:
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c

> @@ -588,12 +590,20 @@ static int cpufreq_get_requested_power(struct 
> thermal_cooling_device *cdev,
>u32 *power)
>  {
> unsigned long freq;
> -   int cpu, ret;
> +   int i = 0, cpu, ret;
> u32 static_power, dynamic_power, total_load = 0;
> struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
> +   u32 *load_cpu = NULL;
>
> freq = cpufreq_quick_get(cpumask_any(_device->allowed_cpus));
>
> +   if (trace_thermal_power_cpu_get_power_enabled()) {

You allocated load_cpu if cpu_get_power trace is enabled.

> +   u32 ncpus = cpumask_weight(_device->allowed_cpus);
> +
> +   load_cpu = devm_kcalloc(>device, ncpus, 
> sizeof(*load_cpu),
> +   GFP_KERNEL);
> +   }
> +
> for_each_cpu(cpu, _device->allowed_cpus) {
> u32 load;
>
> @@ -603,14 +613,29 @@ static int cpufreq_get_requested_power(struct 
> thermal_cooling_device *cdev,
> load = 0;
>
> total_load += load;
> +   if (trace_thermal_power_cpu_limit_enabled() && load_cpu)

But you store values to it only if cpu_limit trace is also enabled,
otherwise it is all zeros.

Why?

[PATCH v3 2/5] thermal: add trace events to the power allocator governor

[Javi Merino]

Add trace events for the power allocator governor and the power actor
interface of the cpu cooling device.

Cc: Zhang Rui 
Cc: Eduardo Valentin 
Cc: Frederic Weisbecker 
Cc: Ingo Molnar 
Acked-by: Steven Rostedt 
Signed-off-by: Javi Merino 
---
 drivers/thermal/cpu_cooling.c  | 31 -
 drivers/thermal/power_allocator.c  | 22 ++-
 include/trace/events/thermal.h | 58 +
 include/trace/events/thermal_power_allocator.h | 87 ++
 4 files changed, 194 insertions(+), 4 deletions(-)
 create mode 100644 include/trace/events/thermal_power_allocator.h

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index ba23150c7bde..c4974144c787 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -31,6 +31,8 @@
 #include 
 #include 
 
+#include 
+
 /*
  * Cooling state <-> CPUFreq frequency
  *
@@ -588,12 +590,20 @@ static int cpufreq_get_requested_power(struct 
thermal_cooling_device *cdev,
   u32 *power)
 {
unsigned long freq;
-   int cpu, ret;
+   int i = 0, cpu, ret;
u32 static_power, dynamic_power, total_load = 0;
struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
+   u32 *load_cpu = NULL;
 
freq = cpufreq_quick_get(cpumask_any(_device->allowed_cpus));
 
+   if (trace_thermal_power_cpu_get_power_enabled()) {
+   u32 ncpus = cpumask_weight(_device->allowed_cpus);
+
+   load_cpu = devm_kcalloc(>device, ncpus, sizeof(*load_cpu),
+   GFP_KERNEL);
+   }
+
for_each_cpu(cpu, _device->allowed_cpus) {
u32 load;
 
@@ -603,14 +613,29 @@ static int cpufreq_get_requested_power(struct 
thermal_cooling_device *cdev,
load = 0;
 
total_load += load;
+   if (trace_thermal_power_cpu_limit_enabled() && load_cpu)
+   load_cpu[i] = load;
+
+   i++;
}
 
cpufreq_device->last_load = total_load;
 
dynamic_power = get_dynamic_power(cpufreq_device, freq);
ret = get_static_power(cpufreq_device, tz, freq, _power);
-   if (ret)
+   if (ret) {
+   if (load_cpu)
+   devm_kfree(>device, load_cpu);
return ret;
+   }
+
+   if (load_cpu) {
+   trace_thermal_power_cpu_get_power(
+   _device->allowed_cpus,
+   freq, load_cpu, i, dynamic_power, static_power);
+
+   devm_kfree(>device, load_cpu);
+   }
 
*power = static_power + dynamic_power;
return 0;
@@ -718,6 +743,8 @@ static int cpufreq_power2state(struct 
thermal_cooling_device *cdev,
return -EINVAL;
}
 
+   trace_thermal_power_cpu_limit(_device->allowed_cpus,
+ target_freq, *state, power);
return 0;
 }
 
diff --git a/drivers/thermal/power_allocator.c 
b/drivers/thermal/power_allocator.c
index 67982d79b76c..3ca7530deac6 100644
--- a/drivers/thermal/power_allocator.c
+++ b/drivers/thermal/power_allocator.c
@@ -19,6 +19,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 #include "thermal_core.h"
 
 #define FRAC_BITS 10
@@ -138,7 +141,14 @@ static u32 pid_controller(struct thermal_zone_device *tz,
/* feed-forward the known sustainable dissipatable power */
power_range = tz->tzp->sustainable_power + frac_to_int(power_range);
 
-   return clamp(power_range, (s64)0, (s64)max_allocatable_power);
+   power_range = clamp(power_range, (s64)0, (s64)max_allocatable_power);
+
+   trace_thermal_power_allocator_pid(tz, frac_to_int(err),
+ frac_to_int(params->err_integral),
+ frac_to_int(p), frac_to_int(i),
+ frac_to_int(d), power_range);
+
+   return power_range;
 }
 
 /**
@@ -219,7 +229,7 @@ static int allocate_power(struct thermal_zone_device *tz,
struct power_allocator_params *params = tz->governor_data;
u32 *req_power, *max_power, *granted_power, *extra_actor_power;
u32 total_req_power, max_allocatable_power;
-   u32 power_range;
+   u32 total_granted_power, power_range;
int i, num_actors, total_weight, ret = 0;
int trip_max_desired_temperature = params->trip_max_desired_temperature;
 
@@ -295,6 +305,7 @@ static int allocate_power(struct thermal_zone_device *tz,
divvy_up_power(req_power, max_power, num_actors, total_req_power,
   power_range, granted_power, extra_actor_power);
 
+   total_granted_power = 0;
i = 0;
list_for_each_entry(instance, >thermal_instances, tz_node) {
if (instance->trip != trip_max_desired_temperature)
@@ -305,10 +316,17 @@ static int allocate_power(struct 

[PATCH v3 2/5] thermal: add trace events to the power allocator governor

[Javi Merino]

Add trace events for the power allocator governor and the power actor
interface of the cpu cooling device.

Cc: Zhang Rui rui.zh...@intel.com
Cc: Eduardo Valentin edubez...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Acked-by: Steven Rostedt rost...@goodmis.org
Signed-off-by: Javi Merino javi.mer...@arm.com
---
 drivers/thermal/cpu_cooling.c  | 31 -
 drivers/thermal/power_allocator.c  | 22 ++-
 include/trace/events/thermal.h | 58 +
 include/trace/events/thermal_power_allocator.h | 87 ++
 4 files changed, 194 insertions(+), 4 deletions(-)
 create mode 100644 include/trace/events/thermal_power_allocator.h

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index ba23150c7bde..c4974144c787 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -31,6 +31,8 @@
 #include linux/cpu.h
 #include linux/cpu_cooling.h
 
+#include trace/events/thermal.h
+
 /*
  * Cooling state - CPUFreq frequency
  *
@@ -588,12 +590,20 @@ static int cpufreq_get_requested_power(struct 
thermal_cooling_device *cdev,
   u32 *power)
 {
unsigned long freq;
-   int cpu, ret;
+   int i = 0, cpu, ret;
u32 static_power, dynamic_power, total_load = 0;
struct cpufreq_cooling_device *cpufreq_device = cdev-devdata;
+   u32 *load_cpu = NULL;
 
freq = cpufreq_quick_get(cpumask_any(cpufreq_device-allowed_cpus));
 
+   if (trace_thermal_power_cpu_get_power_enabled()) {
+   u32 ncpus = cpumask_weight(cpufreq_device-allowed_cpus);
+
+   load_cpu = devm_kcalloc(cdev-device, ncpus, sizeof(*load_cpu),
+   GFP_KERNEL);
+   }
+
for_each_cpu(cpu, cpufreq_device-allowed_cpus) {
u32 load;
 
@@ -603,14 +613,29 @@ static int cpufreq_get_requested_power(struct 
thermal_cooling_device *cdev,
load = 0;
 
total_load += load;
+   if (trace_thermal_power_cpu_limit_enabled()  load_cpu)
+   load_cpu[i] = load;
+
+   i++;
}
 
cpufreq_device-last_load = total_load;
 
dynamic_power = get_dynamic_power(cpufreq_device, freq);
ret = get_static_power(cpufreq_device, tz, freq, static_power);
-   if (ret)
+   if (ret) {
+   if (load_cpu)
+   devm_kfree(cdev-device, load_cpu);
return ret;
+   }
+
+   if (load_cpu) {
+   trace_thermal_power_cpu_get_power(
+   cpufreq_device-allowed_cpus,
+   freq, load_cpu, i, dynamic_power, static_power);
+
+   devm_kfree(cdev-device, load_cpu);
+   }
 
*power = static_power + dynamic_power;
return 0;
@@ -718,6 +743,8 @@ static int cpufreq_power2state(struct 
thermal_cooling_device *cdev,
return -EINVAL;
}
 
+   trace_thermal_power_cpu_limit(cpufreq_device-allowed_cpus,
+ target_freq, *state, power);
return 0;
 }
 
diff --git a/drivers/thermal/power_allocator.c 
b/drivers/thermal/power_allocator.c
index 67982d79b76c..3ca7530deac6 100644
--- a/drivers/thermal/power_allocator.c
+++ b/drivers/thermal/power_allocator.c
@@ -19,6 +19,9 @@
 #include linux/slab.h
 #include linux/thermal.h
 
+#define CREATE_TRACE_POINTS
+#include trace/events/thermal_power_allocator.h
+
 #include thermal_core.h
 
 #define FRAC_BITS 10
@@ -138,7 +141,14 @@ static u32 pid_controller(struct thermal_zone_device *tz,
/* feed-forward the known sustainable dissipatable power */
power_range = tz-tzp-sustainable_power + frac_to_int(power_range);
 
-   return clamp(power_range, (s64)0, (s64)max_allocatable_power);
+   power_range = clamp(power_range, (s64)0, (s64)max_allocatable_power);
+
+   trace_thermal_power_allocator_pid(tz, frac_to_int(err),
+ frac_to_int(params-err_integral),
+ frac_to_int(p), frac_to_int(i),
+ frac_to_int(d), power_range);
+
+   return power_range;
 }
 
 /**
@@ -219,7 +229,7 @@ static int allocate_power(struct thermal_zone_device *tz,
struct power_allocator_params *params = tz-governor_data;
u32 *req_power, *max_power, *granted_power, *extra_actor_power;
u32 total_req_power, max_allocatable_power;
-   u32 power_range;
+   u32 total_granted_power, power_range;
int i, num_actors, total_weight, ret = 0;
int trip_max_desired_temperature = params-trip_max_desired_temperature;
 
@@ -295,6 +305,7 @@ static int allocate_power(struct thermal_zone_device *tz,
divvy_up_power(req_power, max_power, num_actors, total_req_power,
   power_range, granted_power,