The EM is going to support runtime modifications of the power data.
Introduce RCU safe mechanism to clean up the old allocated EM data.
It also adds a mutex for the EM structure to serialize the modifiers.
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
kernel/power/energy_model.c | 42 +++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index c2f8a0046f8a..4596bfe7398e 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -23,6 +23,9 @@
*/
static DEFINE_MUTEX(em_pd_mutex);
+static void em_cpufreq_update_efficiencies(struct device *dev,
+ struct em_perf_state *table);
+
static bool _is_cpu_device(struct device *dev)
{
return (dev->bus == &cpu_subsys);
@@ -104,6 +107,45 @@ static void em_debug_create_pd(struct device *dev) {}
static void em_debug_remove_pd(struct device *dev) {}
#endif
+static void em_destroy_rt_table_rcu(struct rcu_head *rp)
+{
+ struct em_perf_table *runtime_table;
+
+ runtime_table = container_of(rp, struct em_perf_table, rcu);
+ kfree(runtime_table->state);
+ kfree(runtime_table);
+}
+
+static void em_destroy_tmp_setup_rcu(struct rcu_head *rp)
+{
+ struct em_perf_table *runtime_table;
+
+ runtime_table = container_of(rp, struct em_perf_table, rcu);
+ kfree(runtime_table);
+}
+
+static void em_perf_runtime_table_set(struct device *dev,
+ struct em_perf_table *runtime_table)
+{
+ struct em_perf_domain *pd = dev->em_pd;
+ struct em_perf_table *tmp;
+
+ tmp = pd->runtime_table;
+
+ rcu_assign_pointer(pd->runtime_table, runtime_table);
+
+ em_cpufreq_update_efficiencies(dev, runtime_table->state);
+
+ /*
+ * Check if the 'state' array is not actually the one from setup.
+ * If it is then don't free it.
+ */
+ if (tmp->state == pd->default_table->state)
+ call_rcu(&tmp->rcu, em_destroy_tmp_setup_rcu);
+ else
+ call_rcu(&tmp->rcu, em_destroy_rt_table_rcu);
+}
+
static int em_compute_costs(struct device *dev, struct em_perf_state *table,
struct em_data_callback *cb, int nr_states,
unsigned long flags)
--
2.25.1On 21/07/2023 17:50, Lukasz Luba wrote:
> The EM is going to support runtime modifications of the power data.
> Introduce RCU safe mechanism to clean up the old allocated EM data.
> It also adds a mutex for the EM structure to serialize the modifiers.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
> kernel/power/energy_model.c | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index c2f8a0046f8a..4596bfe7398e 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -23,6 +23,9 @@
> */
> static DEFINE_MUTEX(em_pd_mutex);
>
> +static void em_cpufreq_update_efficiencies(struct device *dev,
> + struct em_perf_state *table);
> +
> static bool _is_cpu_device(struct device *dev)
> {
> return (dev->bus == &cpu_subsys);
> @@ -104,6 +107,45 @@ static void em_debug_create_pd(struct device *dev) {}
> static void em_debug_remove_pd(struct device *dev) {}
> #endif
>
> +static void em_destroy_rt_table_rcu(struct rcu_head *rp)
> +{
> + struct em_perf_table *runtime_table;
> +
> + runtime_table = container_of(rp, struct em_perf_table, rcu);
> + kfree(runtime_table->state);
> + kfree(runtime_table);
> +}
> +
> +static void em_destroy_tmp_setup_rcu(struct rcu_head *rp)
> +{
> + struct em_perf_table *runtime_table;
> +
> + runtime_table = container_of(rp, struct em_perf_table, rcu);
> + kfree(runtime_table);
> +}
Still don't like that we have to have 2 rcu callbacks here. In case we
could assign default_table to runtime_table in em_create_pd() (and not
just default_table->state to runtime_table->state) IMHO we would only
need one rcu callback?
-->8--
-static void em_destroy_tmp_setup_rcu(struct rcu_head *rp)
-{
- struct em_perf_table *runtime_table;
-
- runtime_table = container_of(rp, struct em_perf_table, rcu);
- kfree(runtime_table);
-}
-
static void em_perf_runtime_table_set(struct device *dev,
struct em_perf_table *runtime_table)
{
@@ -136,13 +128,8 @@ static void em_perf_runtime_table_set(struct device *dev,
em_cpufreq_update_efficiencies(dev, runtime_table->state);
- /*
- * Check if the 'state' array is not actually the one from setup.
- * If it is then don't free it.
- */
- if (tmp->state == pd->default_table->state)
- call_rcu(&tmp->rcu, em_destroy_tmp_setup_rcu);
- else
+ /* Don't free default table (inital value of runtime table) */
+ if (tmp != pd->default_table)
call_rcu(&tmp->rcu, em_destroy_rt_table_rcu);
}
@@ -349,7 +336,6 @@ static int em_create_pd(struct device *dev, int nr_states,
unsigned long flags)
{
struct em_perf_table *default_table;
- struct em_perf_table *runtime_table;
struct em_perf_domain *pd;
struct device *cpu_dev;
int cpu, ret, num_cpus;
@@ -382,24 +368,15 @@ static int em_create_pd(struct device *dev, int nr_states,
pd->default_table = default_table;
- runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
- if (!runtime_table) {
- kfree(default_table);
- kfree(pd);
- return -ENOMEM;
- }
-
ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
if (ret) {
kfree(default_table);
- kfree(runtime_table);
kfree(pd);
return ret;
}
- /* Re-use temporally (till 1st modification) the memory */
- runtime_table->state = default_table->state;
- rcu_assign_pointer(pd->runtime_table, runtime_table);
+ /* Initialize runtime table as default table */
+ rcu_assign_pointer(pd->runtime_table, default_table);
if (_is_cpu_device(dev))
for_each_cpu(cpu, cpus) {
On 8/16/23 14:06, Dietmar Eggemann wrote:
> On 21/07/2023 17:50, Lukasz Luba wrote:
>> The EM is going to support runtime modifications of the power data.
>> Introduce RCU safe mechanism to clean up the old allocated EM data.
>> It also adds a mutex for the EM structure to serialize the modifiers.
>>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>> kernel/power/energy_model.c | 42 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
>> index c2f8a0046f8a..4596bfe7398e 100644
>> --- a/kernel/power/energy_model.c
>> +++ b/kernel/power/energy_model.c
>> @@ -23,6 +23,9 @@
>> */
>> static DEFINE_MUTEX(em_pd_mutex);
>>
>> +static void em_cpufreq_update_efficiencies(struct device *dev,
>> + struct em_perf_state *table);
>> +
>> static bool _is_cpu_device(struct device *dev)
>> {
>> return (dev->bus == &cpu_subsys);
>> @@ -104,6 +107,45 @@ static void em_debug_create_pd(struct device *dev) {}
>> static void em_debug_remove_pd(struct device *dev) {}
>> #endif
>>
>> +static void em_destroy_rt_table_rcu(struct rcu_head *rp)
>> +{
>> + struct em_perf_table *runtime_table;
>> +
>> + runtime_table = container_of(rp, struct em_perf_table, rcu);
>> + kfree(runtime_table->state);
>> + kfree(runtime_table);
>> +}
>> +
>> +static void em_destroy_tmp_setup_rcu(struct rcu_head *rp)
>> +{
>> + struct em_perf_table *runtime_table;
>> +
>> + runtime_table = container_of(rp, struct em_perf_table, rcu);
>> + kfree(runtime_table);
>> +}
>
> Still don't like that we have to have 2 rcu callbacks here. In case we
> could assign default_table to runtime_table in em_create_pd() (and not
> just default_table->state to runtime_table->state) IMHO we would only
> need one rcu callback?
You have convinced me. I'll change that code.
>
> -->8--
>
> -static void em_destroy_tmp_setup_rcu(struct rcu_head *rp)
> -{
> - struct em_perf_table *runtime_table;
> -
> - runtime_table = container_of(rp, struct em_perf_table, rcu);
> - kfree(runtime_table);
> -}
> -
> static void em_perf_runtime_table_set(struct device *dev,
> struct em_perf_table *runtime_table)
> {
> @@ -136,13 +128,8 @@ static void em_perf_runtime_table_set(struct device *dev,
>
> em_cpufreq_update_efficiencies(dev, runtime_table->state);
>
> - /*
> - * Check if the 'state' array is not actually the one from setup.
> - * If it is then don't free it.
> - */
> - if (tmp->state == pd->default_table->state)
> - call_rcu(&tmp->rcu, em_destroy_tmp_setup_rcu);
> - else
> + /* Don't free default table (inital value of runtime table) */
> + if (tmp != pd->default_table)
> call_rcu(&tmp->rcu, em_destroy_rt_table_rcu);
> }
>
> @@ -349,7 +336,6 @@ static int em_create_pd(struct device *dev, int nr_states,
> unsigned long flags)
> {
> struct em_perf_table *default_table;
> - struct em_perf_table *runtime_table;
> struct em_perf_domain *pd;
> struct device *cpu_dev;
> int cpu, ret, num_cpus;
> @@ -382,24 +368,15 @@ static int em_create_pd(struct device *dev, int nr_states,
>
> pd->default_table = default_table;
>
> - runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
> - if (!runtime_table) {
> - kfree(default_table);
> - kfree(pd);
> - return -ENOMEM;
> - }
> -
> ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
> if (ret) {
> kfree(default_table);
> - kfree(runtime_table);
> kfree(pd);
> return ret;
> }
>
> - /* Re-use temporally (till 1st modification) the memory */
> - runtime_table->state = default_table->state;
> - rcu_assign_pointer(pd->runtime_table, runtime_table);
> + /* Initialize runtime table as default table */
> + rcu_assign_pointer(pd->runtime_table, default_table);
>
> if (_is_cpu_device(dev))
> for_each_cpu(cpu, cpus) {
>
>
>
I'll have to modify the unregister function to be aligned with this
approach as well.
© 2016 - 2025 Red Hat, Inc.