| Message ID | 20231129110853.94344-11-lukasz.luba@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce runtime modifiable Energy Model | expand |
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. > + > +/** > + * 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? Cheers -- Qais Yousef > + > return table; > } > > -- > 2.25.1 >
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!
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); +} + +/** + * 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); + return table; }
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(-)