Messages in this thread | | | Date | Mon, 3 Jul 2023 16:49:10 +0100 | Subject | Re: [PATCH v2 09/17] PM: EM: Add RCU mechanism which safely cleans the old data | From | Lukasz Luba <> |
| |
On 5/30/23 11:02, Dietmar Eggemann wrote: > On 12/05/2023 11:57, Lukasz Luba wrote: >> The EM is going to support runtime modifications of the power data. >> In order to achieve that prepare the internal mechanism. This patch >> introduces RCU safe mechanism to clean up the old allocated EM data. > > s/In order to achieve that prepare the internal mechanism. This patch > introduces/Introduce ... much shorter, same information.
OK
> > [...] > >> +static void em_perf_runtime_table_set(struct device *dev, >> + struct em_perf_table *runtime_table) >> +{ >> + struct em_perf_domain *pd = dev->em_pd; >> + struct em_perf_table *tmp; >> + >> + tmp = pd->runtime_table; >> + >> + rcu_assign_pointer(pd->runtime_table, runtime_table); >> + >> + em_cpufreq_update_efficiencies(dev, runtime_table->state); >> + >> + if (trace_em_perf_state_enabled()) { >> + unsigned long freq, power, cost, flags; >> + int i; >> + >> + for (i = 0; i < pd->nr_perf_states; i++) { >> + freq = runtime_table->state[i].frequency; >> + power = runtime_table->state[i].power; >> + cost = runtime_table->state[i].cost; >> + flags = runtime_table->state[i].flags; >> + >> + trace_em_perf_state(dev_name(dev), pd->nr_perf_states, >> + i, freq, power, cost, flags); >> + } >> + } >> + >> + /* >> + * Check if the 'state' array is not actually the one from setup. >> + * If it is then don't free it. >> + */ >> + if (tmp->state == pd->table) > > It's unfortunate that you have the refactoring in 13/17 which would lead to: > > if (pd->runtime_table>state == pd->default_table->state) > ^^^^^^^^^^^^^ ^^^^^^^^^^^^^ > > so people would immediately grasp one of the core concepts of the > design: non-modifiable default_table and modifiable runtime_table. > > I still belief that it would be the better idea to do the refactoring first. > > [...]
That make sense. Let me try that approach as see how the patch set would look like.
| |