diff mbox series

[v7,12/23] PM: EM: Add em_perf_state_from_pd() to get performance states table

Message ID 20240117095714.1524808-13-lukasz.luba@arm.com (mailing list archive)
State Superseded, archived
Headers show
Series Introduce runtime modifiable Energy Model | expand

Commit Message

Lukasz Luba Jan. 17, 2024, 9:57 a.m. UTC
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(+)

Comments

Dietmar Eggemann Jan. 29, 2024, 6:13 p.m. UTC | #1
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?

[...]
Lukasz Luba Feb. 6, 2024, 1:55 p.m. UTC | #2
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 mbox series

Patch

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