There is going to be a new update function addressing chip binning.
Therefore, some common code which can be refactored and called from
upcoming changes and em_adjust_new_capacity(). In this way the code
duplication can be avoided.
Signed-off-by: Lukasz Luba <[email protected]>
---
kernel/power/energy_model.c | 58 +++++++++++++++++++++++++------------
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 9e1c9aa399ea9..6960dd7393b2d 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -674,23 +674,15 @@ void em_dev_unregister_perf_domain(struct device *dev)
}
EXPORT_SYMBOL_GPL(em_dev_unregister_perf_domain);
-/*
- * Adjustment of CPU performance values after boot, when all CPUs capacites
- * are correctly calculated.
- */
-static void em_adjust_new_capacity(struct device *dev,
- struct em_perf_domain *pd,
- u64 max_cap)
+static struct em_perf_table __rcu *em_table_dup(struct em_perf_domain *pd)
{
struct em_perf_table __rcu *em_table;
struct em_perf_state *ps, *new_ps;
- int ret, ps_size;
+ int ps_size;
em_table = em_table_alloc(pd);
- if (!em_table) {
- dev_warn(dev, "EM: allocation failed\n");
- return;
- }
+ if (!em_table)
+ return NULL;
new_ps = em_table->state;
@@ -702,24 +694,52 @@ static void em_adjust_new_capacity(struct device *dev,
rcu_read_unlock();
- em_init_performance(dev, pd, new_ps, pd->nr_perf_states);
- ret = em_compute_costs(dev, new_ps, NULL, pd->nr_perf_states,
+ return em_table;
+}
+
+static int em_recalc_and_update(struct device *dev, struct em_perf_domain *pd,
+ struct em_perf_table __rcu *em_table)
+{
+ int ret;
+
+ ret = em_compute_costs(dev, em_table->state, NULL, pd->nr_perf_states,
pd->flags);
- if (ret) {
- dev_warn(dev, "EM: compute costs failed\n");
- return;
- }
+ if (ret)
+ goto free_em_table;
ret = em_dev_update_perf_domain(dev, em_table);
if (ret)
- dev_warn(dev, "EM: update failed %d\n", ret);
+ goto free_em_table;
/*
* This is one-time-update, so give up the ownership in this updater.
* The EM framework has incremented the usage counter and from now
* will keep the reference (then free the memory when needed).
*/
+free_em_table:
em_table_free(em_table);
+ return ret;
+}
+
+/*
+ * Adjustment of CPU performance values after boot, when all CPUs capacites
+ * are correctly calculated.
+ */
+static void em_adjust_new_capacity(struct device *dev,
+ struct em_perf_domain *pd,
+ u64 max_cap)
+{
+ struct em_perf_table __rcu *em_table;
+
+ em_table = em_table_dup(pd);
+ if (!em_table) {
+ dev_warn(dev, "EM: allocation failed\n");
+ return;
+ }
+
+ em_init_performance(dev, pd, em_table->state, pd->nr_perf_states);
+
+ em_recalc_and_update(dev, pd, em_table);
}
static void em_check_capacity_update(void)
--
2.25.1
On 22/03/2024 12:08, Lukasz Luba wrote:
Maybe better : "PM: EM: Refactoring em_adjust_new_capacity()" ?
> There is going to be a new update function addressing chip binning.
> Therefore, some common code which can be refactored and called from
> upcoming changes and em_adjust_new_capacity(). In this way the code
> duplication can be avoided.
IMHO, that's hard to digest.
Extract em_table_dup() and em_recalc_and_update() from
em_adjust_new_capacity(). Both functions will be later reused by the
'update EM due to chip binning' functionality.
[...]
> +static int em_recalc_and_update(struct device *dev, struct em_perf_domain *pd,
> + struct em_perf_table __rcu *em_table)
> +{
> + int ret;
> +
> + ret = em_compute_costs(dev, em_table->state, NULL, pd->nr_perf_states,
> pd->flags);
> - if (ret) {
> - dev_warn(dev, "EM: compute costs failed\n");
> - return;
> - }
> + if (ret)
> + goto free_em_table;
There seems to be a subtle change in this patch. When em_compute_costs()
fails now em_table_free() is called. This wasn't the case before when
em_compute_costs() was directly called from em_adjust_new_capacity().
[...]
On 3/26/24 10:51, Dietmar Eggemann wrote:
> On 22/03/2024 12:08, Lukasz Luba wrote:
>
> Maybe better : "PM: EM: Refactoring em_adjust_new_capacity()" ?
>
>> There is going to be a new update function addressing chip binning.
>> Therefore, some common code which can be refactored and called from
>> upcoming changes and em_adjust_new_capacity(). In this way the code
>> duplication can be avoided.
>
> IMHO, that's hard to digest.
>
> Extract em_table_dup() and em_recalc_and_update() from
> em_adjust_new_capacity(). Both functions will be later reused by the
> 'update EM due to chip binning' functionality.
That looks good, I'll update it.
>
> [...]
>
>> +static int em_recalc_and_update(struct device *dev, struct em_perf_domain *pd,
>> + struct em_perf_table __rcu *em_table)
>> +{
>> + int ret;
>> +
>> + ret = em_compute_costs(dev, em_table->state, NULL, pd->nr_perf_states,
>> pd->flags);
>> - if (ret) {
>> - dev_warn(dev, "EM: compute costs failed\n");
>> - return;
>> - }
>> + if (ret)
>> + goto free_em_table;
>
> There seems to be a subtle change in this patch. When em_compute_costs()
> fails now em_table_free() is called. This wasn't the case before when
> em_compute_costs() was directly called from em_adjust_new_capacity().
Yes, I've refactored it to explicitly call to the same free_em_table
for both fails in the new code. It should have been done in old code.
>
> [...]