Hi all,
This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
cooling code.
Changes:
v2:
- extened the cpufreq_cooling_device with private ops field, to not waste
memory and simplify allocation/free code (Viresh)
- added devfreq_cooling clean-up to align with cpufreq cooling code
- added ACKs from Viresh for patch 2/4 and path 3/4
- added missing maintainers of tracing to CC list
Regards,
Lukasz
Lukasz Luba (4):
thermal: cpufreq_cooling: Use private callback ops for each cooling
device
thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
tracing
thermal: cpufreq_cooling: Update outdated comments
thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
drivers/thermal/devfreq_cooling.c | 27 ++++-------
include/trace/events/thermal.h | 28 ++++-------
3 files changed, 42 insertions(+), 90 deletions(-)
--
2.17.1
Remove unneeded global variable devfreq_cooling_ops which is used only
as a copy pattern. Instead, extend the struct devfreq_cooling_device with
the needed ops structure. This also simplifies the allocation/free code
during the setup/cleanup.
Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/devfreq_cooling.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 8c76f9655e57..67b618b1afc8 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -28,6 +28,7 @@
* struct devfreq_cooling_device - Devfreq cooling device
* devfreq_cooling_device registered.
* @cdev: Pointer to associated thermal cooling device.
+ * @cooling_ops: devfreq callbacks to thermal cooling device ops
* @devfreq: Pointer to associated devfreq device.
* @cooling_state: Current cooling state.
* @freq_table: Pointer to a table with the frequencies sorted in descending
@@ -48,6 +49,7 @@
*/
struct devfreq_cooling_device {
struct thermal_cooling_device *cdev;
+ struct thermal_cooling_device_ops cooling_ops;
struct devfreq *devfreq;
unsigned long cooling_state;
u32 *freq_table;
@@ -290,12 +292,6 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
return 0;
}
-static struct thermal_cooling_device_ops devfreq_cooling_ops = {
- .get_max_state = devfreq_cooling_get_max_state,
- .get_cur_state = devfreq_cooling_get_cur_state,
- .set_cur_state = devfreq_cooling_set_cur_state,
-};
-
/**
* devfreq_cooling_gen_tables() - Generate frequency table.
* @dfc: Pointer to devfreq cooling device.
@@ -363,18 +359,18 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
char *name;
int err, num_opps;
- ops = kmemdup(&devfreq_cooling_ops, sizeof(*ops), GFP_KERNEL);
- if (!ops)
- return ERR_PTR(-ENOMEM);
dfc = kzalloc(sizeof(*dfc), GFP_KERNEL);
- if (!dfc) {
- err = -ENOMEM;
- goto free_ops;
- }
+ if (!dfc)
+ return ERR_PTR(-ENOMEM);
dfc->devfreq = df;
+ ops = &dfc->cooling_ops;
+ ops->get_max_state = devfreq_cooling_get_max_state;
+ ops->get_cur_state = devfreq_cooling_get_cur_state;
+ ops->set_cur_state = devfreq_cooling_set_cur_state;
+
em = em_pd_get(dev);
if (em && !em_is_artificial(em)) {
dfc->em_pd = em;
@@ -437,8 +433,6 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df,
kfree(dfc->freq_table);
free_dfc:
kfree(dfc);
-free_ops:
- kfree(ops);
return ERR_PTR(err);
}
@@ -520,13 +514,11 @@ EXPORT_SYMBOL_GPL(devfreq_cooling_em_register);
void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
{
struct devfreq_cooling_device *dfc;
- const struct thermal_cooling_device_ops *ops;
struct device *dev;
if (IS_ERR_OR_NULL(cdev))
return;
- ops = cdev->ops;
dfc = cdev->devdata;
dev = dfc->devfreq->dev.parent;
@@ -537,6 +529,5 @@ void devfreq_cooling_unregister(struct thermal_cooling_device *cdev)
kfree(dfc->freq_table);
kfree(dfc);
- kfree(ops);
}
EXPORT_SYMBOL_GPL(devfreq_cooling_unregister);
--
2.17.1
Simplify the thermal_power_cpu_get_power trace event by removing
complicated cpumask and variable length array. Now the tools parsing trace
output don't have to hassle to get this power data. The simplified format
version uses 'policy->cpu'. Remove also the 'load' information completely
since there is very little value of it in this trace event. To get the
CPUs' load (or utilization) there are other dedicated trace hooks in the
kernel. This patch also simplifies and speeds-up the main cooling code
when that trace event is enabled.
Rename the trace event to avoid confusion of tools which parse the trace
file.
Acked-by: Viresh Kumar <[email protected]>
Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/cpufreq_cooling.c | 18 +-----------------
include/trace/events/thermal.h | 28 ++++++++--------------------
2 files changed, 9 insertions(+), 37 deletions(-)
diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index ad8b86f5281b..492a67e267e8 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -216,16 +216,9 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
u32 total_load = 0;
struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
struct cpufreq_policy *policy = cpufreq_cdev->policy;
- u32 *load_cpu = NULL;
freq = cpufreq_quick_get(policy->cpu);
- if (trace_thermal_power_cpu_get_power_enabled()) {
- u32 ncpus = cpumask_weight(policy->related_cpus);
-
- load_cpu = kcalloc(ncpus, sizeof(*load_cpu), GFP_KERNEL);
- }
-
for_each_cpu(cpu, policy->related_cpus) {
u32 load;
@@ -235,22 +228,13 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
load = 0;
total_load += load;
- if (load_cpu)
- load_cpu[i] = load;
-
- i++;
}
cpufreq_cdev->last_load = total_load;
*power = get_dynamic_power(cpufreq_cdev, freq);
- if (load_cpu) {
- trace_thermal_power_cpu_get_power(policy->related_cpus, freq,
- load_cpu, i, *power);
-
- kfree(load_cpu);
- }
+ trace_thermal_power_cpu_get_power_simple(policy->cpu, *power);
return 0;
}
diff --git a/include/trace/events/thermal.h b/include/trace/events/thermal.h
index 8a5f04888abd..e58bf3072f32 100644
--- a/include/trace/events/thermal.h
+++ b/include/trace/events/thermal.h
@@ -92,34 +92,22 @@ TRACE_EVENT(thermal_zone_trip,
);
#ifdef CONFIG_CPU_THERMAL
-TRACE_EVENT(thermal_power_cpu_get_power,
- TP_PROTO(const struct cpumask *cpus, unsigned long freq, u32 *load,
- size_t load_len, u32 dynamic_power),
+TRACE_EVENT(thermal_power_cpu_get_power_simple,
+ TP_PROTO(int cpu, u32 power),
- TP_ARGS(cpus, freq, load, load_len, dynamic_power),
+ TP_ARGS(cpu, power),
TP_STRUCT__entry(
- __bitmask(cpumask, num_possible_cpus())
- __field(unsigned long, freq )
- __dynamic_array(u32, load, load_len)
- __field(size_t, load_len )
- __field(u32, dynamic_power )
+ __field(int, cpu)
+ __field(u32, power)
),
TP_fast_assign(
- __assign_bitmask(cpumask, cpumask_bits(cpus),
- num_possible_cpus());
- __entry->freq = freq;
- memcpy(__get_dynamic_array(load), load,
- load_len * sizeof(*load));
- __entry->load_len = load_len;
- __entry->dynamic_power = dynamic_power;
+ __entry->cpu = cpu;
+ __entry->power = power;
),
- TP_printk("cpus=%s freq=%lu load={%s} dynamic_power=%d",
- __get_bitmask(cpumask), __entry->freq,
- __print_array(__get_dynamic_array(load), __entry->load_len, 4),
- __entry->dynamic_power)
+ TP_printk("cpu=%d power=%u", __entry->cpu, __entry->power)
);
TRACE_EVENT(thermal_power_cpu_limit,
--
2.17.1
Hi Daniel,
On 6/13/22 13:43, Lukasz Luba wrote:
> Hi all,
>
> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
> cooling code.
>
> Changes:
> v2:
> - extened the cpufreq_cooling_device with private ops field, to not waste
> memory and simplify allocation/free code (Viresh)
> - added devfreq_cooling clean-up to align with cpufreq cooling code
> - added ACKs from Viresh for patch 2/4 and path 3/4
> - added missing maintainers of tracing to CC list
>
> Regards,
> Lukasz
>
> Lukasz Luba (4):
> thermal: cpufreq_cooling: Use private callback ops for each cooling
> device
> thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
> tracing
> thermal: cpufreq_cooling: Update outdated comments
> thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
>
> drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
> drivers/thermal/devfreq_cooling.c | 27 ++++-------
> include/trace/events/thermal.h | 28 ++++-------
> 3 files changed, 42 insertions(+), 90 deletions(-)
>
Could you have a look and take the patches into your tree, please?
The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
is pretty minor change.
Regards,
Lukasz
On 14/06/2022 14:51, Lukasz Luba wrote:
> Hi Daniel,
>
>
> On 6/13/22 13:43, Lukasz Luba wrote:
>> Hi all,
>>
>> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
>> cooling code.
>>
>> Changes:
>> v2:
>> - extened the cpufreq_cooling_device with private ops field, to not waste
>> memory and simplify allocation/free code (Viresh)
>> - added devfreq_cooling clean-up to align with cpufreq cooling code
>> - added ACKs from Viresh for patch 2/4 and path 3/4
>> - added missing maintainers of tracing to CC list
>>
>> Regards,
>> Lukasz
>>
>> Lukasz Luba (4):
>> thermal: cpufreq_cooling: Use private callback ops for each cooling
>> device
>> thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
>> tracing
>> thermal: cpufreq_cooling: Update outdated comments
>> thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
>>
>> drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
>> drivers/thermal/devfreq_cooling.c | 27 ++++-------
>> include/trace/events/thermal.h | 28 ++++-------
>> 3 files changed, 42 insertions(+), 90 deletions(-)
>>
>
> Could you have a look and take the patches into your tree, please?
> The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
> is pretty minor change.
Applied, thanks
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 6/14/22 19:26, Daniel Lezcano wrote:
> On 14/06/2022 14:51, Lukasz Luba wrote:
>> Hi Daniel,
>>
>>
>> On 6/13/22 13:43, Lukasz Luba wrote:
>>> Hi all,
>>>
>>> This is v2 of some minor clean-ups for the thermal cpufreq and devfreq
>>> cooling code.
>>>
>>> Changes:
>>> v2:
>>> - extened the cpufreq_cooling_device with private ops field, to not
>>> waste
>>> memory and simplify allocation/free code (Viresh)
>>> - added devfreq_cooling clean-up to align with cpufreq cooling code
>>> - added ACKs from Viresh for patch 2/4 and path 3/4
>>> - added missing maintainers of tracing to CC list
>>>
>>> Regards,
>>> Lukasz
>>>
>>> Lukasz Luba (4):
>>> thermal: cpufreq_cooling: Use private callback ops for each cooling
>>> device
>>> thermal: cpufreq_cooling : Refactor thermal_power_cpu_get_power
>>> tracing
>>> thermal: cpufreq_cooling: Update outdated comments
>>> thermal: devfreq_cooling: Extend the devfreq_cooling_device with ops
>>>
>>> drivers/thermal/cpufreq_cooling.c | 77 ++++++++++---------------------
>>> drivers/thermal/devfreq_cooling.c | 27 ++++-------
>>> include/trace/events/thermal.h | 28 ++++-------
>>> 3 files changed, 42 insertions(+), 90 deletions(-)
>>>
>>
>> Could you have a look and take the patches into your tree, please?
>> The 3 of 4 patches got ACKs, the last one is devfreq cooling, which
>> is pretty minor change.
>
> Applied, thanks
>
>
Thanks Daniel!