Messages in this thread | | | Date | Tue, 19 Dec 2023 08:45:04 +0000 | Subject | Re: [PATCH v5 10/23] PM: EM: Add API for memory allocations for new tables | From | Lukasz Luba <> |
| |
On 12/17/23 17:59, Qais Yousef wrote: > On 11/29/23 11:08, Lukasz Luba wrote: >> The runtime modified EM table can be provided from drivers. Create >> mechanism which allows safely allocate and free the table for device >> drivers. The same table can be used by the EAS in task scheduler code >> paths, so make sure the memory is not freed when the device driver module >> is unloaded. >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> include/linux/energy_model.h | 11 +++++++++ >> kernel/power/energy_model.c | 44 ++++++++++++++++++++++++++++++++++-- >> 2 files changed, 53 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h >> index 94a77a813724..e785211828fe 100644 >> --- a/include/linux/energy_model.h >> +++ b/include/linux/energy_model.h >> @@ -5,6 +5,7 @@ >> #include <linux/device.h> >> #include <linux/jump_label.h> >> #include <linux/kobject.h> >> +#include <linux/kref.h> >> #include <linux/rcupdate.h> >> #include <linux/sched/cpufreq.h> >> #include <linux/sched/topology.h> >> @@ -39,10 +40,12 @@ struct em_perf_state { >> /** >> * struct em_perf_table - Performance states table >> * @rcu: RCU used for safe access and destruction >> + * @refcount: Reference count to track the owners >> * @state: List of performance states, in ascending order >> */ >> struct em_perf_table { >> struct rcu_head rcu; >> + struct kref refcount; >> struct em_perf_state state[]; >> }; >> >> @@ -184,6 +187,8 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states, >> struct em_data_callback *cb, cpumask_t *span, >> bool microwatts); >> void em_dev_unregister_perf_domain(struct device *dev); >> +struct em_perf_table __rcu *em_allocate_table(struct em_perf_domain *pd); >> +void em_free_table(struct em_perf_table __rcu *table); >> >> /** >> * em_pd_get_efficient_state() - Get an efficient performance state from the EM >> @@ -368,6 +373,12 @@ static inline int em_pd_nr_perf_states(struct em_perf_domain *pd) >> { >> return 0; >> } >> +static inline >> +struct em_perf_table __rcu *em_allocate_table(struct em_perf_domain *pd) >> +{ >> + return NULL; >> +} >> +static inline void em_free_table(struct em_perf_table __rcu *table) {} >> #endif >> >> #endif >> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c >> index 489287666705..489a358b9a00 100644 >> --- a/kernel/power/energy_model.c >> +++ b/kernel/power/energy_model.c >> @@ -114,12 +114,46 @@ static void em_destroy_table_rcu(struct rcu_head *rp) >> kfree(runtime_table); >> } >> >> -static void em_free_table(struct em_perf_table __rcu *table) >> +static void em_release_table_kref(struct kref *kref) >> { >> + struct em_perf_table __rcu *table; >> + >> + /* It was the last owner of this table so we can free */ >> + table = container_of(kref, struct em_perf_table, refcount); >> + >> call_rcu(&table->rcu, em_destroy_table_rcu); >> } >> >> -static struct em_perf_table __rcu * >> +static inline void em_inc_usage(struct em_perf_table __rcu *table) >> +{ >> + kref_get(&table->refcount); >> +} >> + >> +static void em_dec_usage(struct em_perf_table __rcu *table) >> +{ >> + kref_put(&table->refcount, em_release_table_kref); >> +} > > nit: em_table_inc/dec() instead? matches general theme elsewhere in the code > base.
Looks good, I will change it.
> >> + >> +/** >> + * em_free_table() - Handles safe free of the EM table when needed >> + * @table : EM memory which is going to be freed >> + * >> + * No return values. >> + */ >> +void em_free_table(struct em_perf_table __rcu *table) >> +{ >> + em_dec_usage(table); >> +} >> + >> +/** >> + * em_allocate_table() - Handles safe allocation of the new EM table >> + * @table : EM memory which is going to be freed >> + * >> + * Increments the reference counter to mark that there is an owner of that >> + * EM table. That might be a device driver module or EAS. >> + * Returns allocated table or error. >> + */ >> +struct em_perf_table __rcu * >> em_allocate_table(struct em_perf_domain *pd) >> { >> struct em_perf_table __rcu *table; >> @@ -128,6 +162,12 @@ em_allocate_table(struct em_perf_domain *pd) >> table_size = sizeof(struct em_perf_state) * pd->nr_perf_states; >> >> table = kzalloc(sizeof(*table) + table_size, GFP_KERNEL); >> + if (!table) >> + return table; >> + >> + kref_init(&table->refcount); >> + em_inc_usage(table); > > Doesn't kref_init() initialize to the count to 1 already? Is the em_inc_usage() > needed here?
Good catch this is not needed here. Thanks!
| |