| Message ID | 20240117095714.1524808-13-lukasz.luba@arm.com |
|---|---|
| State | Accepted |
| Commit | ee1a19873ce1234a3c2e6f84af3624fc73bfbd9c |
| Headers | show |
| Series | Introduce runtime modifiable Energy Model | expand |
On 17/01/2024 10:57, Lukasz Luba wrote: [...] > diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h > index 494df6942cf7..5ebe9dbec8e1 100644 > --- a/include/linux/energy_model.h > +++ b/include/linux/energy_model.h > @@ -339,6 +339,23 @@ static inline int em_pd_nr_perf_states(struct em_perf_domain *pd) > return pd->nr_perf_states; > } > > +/** > + * em_perf_state_from_pd() - Get the performance states table of perf. > + * domain > + * @pd : performance domain for which this must be done > + * > + * To use this function the rcu_read_lock() should be hold. After the usage > + * of the performance states table is finished, the rcu_read_unlock() should > + * be called. > + * > + * Return: the pointer to performance states table of the performance domain > + */ > +static inline > +struct em_perf_state *em_perf_state_from_pd(struct em_perf_domain *pd) This is IMHO hard to get since: struct em_perf_table { struct rcu_head rcu; struct kref kref; struct em_perf_state state[]; }; So very often a 'struct em_perf_table' is named 'table' and 'struct em_perf_table::state' as well. E.g. in em_adjust_new_capacity(). struct em_perf_state *new_table; new_table = em_table->state; In older EM code, we used 'struct em_perf_state *ps' to avoid this confusion, I guess. And what you get from the PD is actually a state vector so maybe: struct em_perf_state *em_get_perf_states(struct em_perf_domain *pd) The 'from_pd' seems obvious because of the parameter? [...]
On 1/29/24 18:13, Dietmar Eggemann wrote: > On 17/01/2024 10:57, Lukasz Luba wrote: > > [...] > >> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h >> index 494df6942cf7..5ebe9dbec8e1 100644 >> --- a/include/linux/energy_model.h >> +++ b/include/linux/energy_model.h >> @@ -339,6 +339,23 @@ static inline int em_pd_nr_perf_states(struct em_perf_domain *pd) >> return pd->nr_perf_states; >> } >> >> +/** >> + * em_perf_state_from_pd() - Get the performance states table of perf. >> + * domain >> + * @pd : performance domain for which this must be done >> + * >> + * To use this function the rcu_read_lock() should be hold. After the usage >> + * of the performance states table is finished, the rcu_read_unlock() should >> + * be called. >> + * >> + * Return: the pointer to performance states table of the performance domain >> + */ >> +static inline >> +struct em_perf_state *em_perf_state_from_pd(struct em_perf_domain *pd) > > This is IMHO hard to get since: > > struct em_perf_table { > struct rcu_head rcu; > struct kref kref; > struct em_perf_state state[]; > }; > > So very often a 'struct em_perf_table' is named 'table' and 'struct > em_perf_table::state' as well. E.g. in em_adjust_new_capacity(). > > struct em_perf_state *new_table; > > new_table = em_table->state; > > In older EM code, we used 'struct em_perf_state *ps' to avoid this > confusion, I guess. > > And what you get from the PD is actually a state vector so maybe: > > struct em_perf_state *em_get_perf_states(struct em_perf_domain *pd) > > The 'from_pd' seems obvious because of the parameter? Rafael proposed that function name in his review comments, so I followed. I might address your comment about the: 'struct em_perf_state *ps' while I will do re-basing and sending v8.
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h index 494df6942cf7..5ebe9dbec8e1 100644 --- a/include/linux/energy_model.h +++ b/include/linux/energy_model.h @@ -339,6 +339,23 @@ static inline int em_pd_nr_perf_states(struct em_perf_domain *pd) return pd->nr_perf_states; } +/** + * em_perf_state_from_pd() - Get the performance states table of perf. + * domain + * @pd : performance domain for which this must be done + * + * To use this function the rcu_read_lock() should be hold. After the usage + * of the performance states table is finished, the rcu_read_unlock() should + * be called. + * + * Return: the pointer to performance states table of the performance domain + */ +static inline +struct em_perf_state *em_perf_state_from_pd(struct em_perf_domain *pd) +{ + return rcu_dereference(pd->em_table)->state; +} + #else struct em_data_callback {}; #define EM_ADV_DATA_CB(_active_power_cb, _cost_cb) { } @@ -385,6 +402,11 @@ int em_dev_update_perf_domain(struct device *dev, { return -EINVAL; } +static inline +struct em_perf_state *em_perf_state_from_pd(struct em_perf_domain *pd) +{ + return NULL; +} #endif #endif
Introduce a wrapper to get the performance states table of the performance domain. The function should be called within the RCU read critical section. Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- include/linux/energy_model.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)