| Message ID | 20240104171553.2080674-1-lukasz.luba@arm.com |
|---|---|
| Headers | show |
| Series | Introduce runtime modifiable Energy Model | expand |
On 1/4/24 19:15, Rafael J. Wysocki wrote: > Here, I would say "Introduce em_compute_costs()" in the subject and then -> OK > > On Thu, Jan 4, 2024 at 6:15 PM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> Refactor a dedicated function which will be easier to maintain and re-use > > -> "Move the EM costs computation code into a new dedicated function, > em_compute_costs(), that can be reused in other places in the future." > OK
On 1/4/24 19:18, Rafael J. Wysocki wrote: > The changelog actually sets what happens here, so why don't you put > that into the changelog too? Something like: "Split the allocation > and initialization of the EM table" OK I will do that. > > On Thu, Jan 4, 2024 at 6:15 PM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> Split the process of allocation and data initialization for the EM table. >> The upcoming changes for modifiable EM will use it. >> >> This change is not expected to alter the general functionality. >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> kernel/power/energy_model.c | 55 ++++++++++++++++++++++--------------- >> 1 file changed, 33 insertions(+), 22 deletions(-) [snip] >> -- > > The code changes LGTM. Thanks
On 1/4/24 19:47, Rafael J. Wysocki wrote: > I don't really like using the API TLA in patch subjects, because it > does not really say much. IMO a subject like this would be better: > > "PM: EM: Introduce em_dev_update_perf_domain() for EM updates" Fair enough > > On Thu, Jan 4, 2024 at 6:15 PM Lukasz Luba <lukasz.luba@arm.com> wrote: >> >> Add API function em_dev_update_perf_domain() which allows to safely >> change the EM. > > "... which allows the EM to be changed safely." OK > > New paragraph: > >> The concurrent modifiers are protected by the mutex >> to serialize them. Removal of the old memory is asynchronous and >> handled by the RCU mechanisms. > > "Concurrent updaters are serialized with a mutex and the removal of > memory that will not be used any more is carried out with the help of > RCU." OK > >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> include/linux/energy_model.h | 8 +++++++ >> kernel/power/energy_model.c | 41 ++++++++++++++++++++++++++++++++++++ >> 2 files changed, 49 insertions(+) >> >> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h >> index 753d70d0ce7e..f33257ed83fd 100644 >> --- a/include/linux/energy_model.h >> +++ b/include/linux/energy_model.h >> @@ -183,6 +183,8 @@ struct em_data_callback { >> >> struct em_perf_domain *em_cpu_get(int cpu); >> struct em_perf_domain *em_pd_get(struct device *dev); >> +int em_dev_update_perf_domain(struct device *dev, >> + struct em_perf_table __rcu *new_table); >> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states, >> struct em_data_callback *cb, cpumask_t *span, >> bool microwatts); >> @@ -376,6 +378,12 @@ 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) {} >> +static inline >> +int em_dev_update_perf_domain(struct device *dev, >> + struct em_perf_table __rcu *new_table) >> +{ >> + return -EINVAL; >> +} >> #endif >> >> #endif >> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c >> index bbc406db0be1..496dc00835c6 100644 >> --- a/kernel/power/energy_model.c >> +++ b/kernel/power/energy_model.c >> @@ -220,6 +220,47 @@ static int em_allocate_perf_table(struct em_perf_domain *pd, >> return 0; >> } >> >> +/** >> + * em_dev_update_perf_domain() - Update runtime EM table for a device >> + * @dev : Device for which the EM is to be updated >> + * @table : The new EM table that is going to be used from now > > This is called "new_table" below. good catch > >> + * >> + * Update EM runtime modifiable table for the @dev using the provided @table. >> + * >> + * This function uses mutex to serialize writers, so it must not be called > > "uses a mutex" OK > >> + * from non-sleeping context. > > "a non-sleeping context". OK > >> + * >> + * Return 0 on success or a proper error in case of failure. > > It is not clear what "a proper error" means. It would be better to > simply say "or an error code on failure" IMO. Agree, I'll change it. > >> + */ >> +int em_dev_update_perf_domain(struct device *dev, >> + struct em_perf_table __rcu *new_table) >> +{ >> + struct em_perf_table __rcu *old_table; >> + struct em_perf_domain *pd; >> + >> + /* Serialize update/unregister or concurrent updates */ >> + mutex_lock(&em_pd_mutex); >> + >> + if (!dev || !dev->em_pd) { > > dev need not be checked under the lock. True, I will put it about the lock. > >> + mutex_unlock(&em_pd_mutex); >> + return -EINVAL; >> + } >> + pd = dev->em_pd; >> + >> + em_table_inc(new_table); >> + >> + old_table = pd->em_table; >> + rcu_assign_pointer(pd->em_table, new_table); >> + >> + em_cpufreq_update_efficiencies(dev, new_table->state); >> + >> + em_table_dec(old_table); >> + >> + mutex_unlock(&em_pd_mutex); >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(em_dev_update_perf_domain); >> + >> static int em_create_runtime_table(struct em_perf_domain *pd) >> { >> struct em_perf_table __rcu *table; >> -- Thank you for the review! Regards, Lukasz
Hi all, This patch set adds a new feature which allows to modify Energy Model (EM) power values at runtime. It will allow to better reflect power model of a recent SoCs and silicon. Different characteristics of the power usage can be leveraged and thus better decisions made during task placement in EAS. It also optimizes the EAS code hot path, by removing 2 division and 1 multiplication operations in the em_cpu_energy(). Speed-up results: the em_cpu_energy() should run faster on the Big CPU by 1.43x and on the Little CPU by 1.69x (mainline board RockPi 4B). This patch set is part of feature set known as Dynamic Energy Model. It has been presented and discussed recently at OSPM2023 [3]. The concepts: 1. The CPU power usage can vary due to the workload that it's running or due to the temperature of the SoC. The same workload can use more power when the temperature of the silicon has increased (e.g. due to hot GPU or ISP). In such situation the EM can be adjusted and reflect the fact of increased power usage. That power increase is due to static power (sometimes called simply: leakage). The CPUs in recent SoCs are different. We have heterogeneous SoCs with 3 (or even 4) different microarchitectures. They are also built differently with High Performance (HP) cells or Low Power (LP) cells. They are affected by the temperature increase differently: HP cells have bigger leakage. The SW model can leverage that knowledge. 2. It is also possible to change the EM to better reflect the currently running workload. Usually the EM is derived from some average power values taken from experiments with benchmark (e.g. Dhrystone). The model derived from such scenario might not represent properly the workloads usually running on the device. Therefore, runtime modification of the EM allows to switch to a different model, when there is a need. 3. The EM can be adjusted after boot, when all the modules are loaded and more information about the SoC is available e.g. chip binning. This would help to better reflect the silicon characteristics. Thus, this EM modification API allows it now. It wasn't possible in the past and the EM had to be 'set in stone'. Example of such runtime modification after boot can be found in a follow-up patch set. It adds the OPP API and usage in Exynos5 SoC driver after the voltage values has been adjusted and power changes [5]. More detailed explanation and background can be found in presentations during LPC2022 [1][2] or in the documentation patches. Some test results: The EM can be updated to fit better the workload type. In the case below the EM has been updated for the Jankbench test on Pixel6 (running v5.18 w/ mainline backports for the scheduler bits). The Jankbench was run 10 times for those two configurations, to get more reliable data. 1. Janky frames percentage +--------+-----------------+---------------------+-------+-----------+ | metric | variable | kernel | value | perc_diff | +--------+-----------------+---------------------+-------+-----------+ | gmean | jank_percentage | EM_default | 2.0 | 0.0% | | gmean | jank_percentage | EM_modified_runtime | 1.3 | -35.33% | +--------+-----------------+---------------------+-------+-----------+ 2. Avg frame render time duration +--------+---------------------+---------------------+-------+-----------+ | metric | variable | kernel | value | perc_diff | +--------+---------------------+---------------------+-------+-----------+ | gmean | mean_frame_duration | EM_default | 10.5 | 0.0% | | gmean | mean_frame_duration | EM_modified_runtime | 9.6 | -8.52% | +--------+---------------------+---------------------+-------+-----------+ 3. Max frame render time duration +--------+--------------------+---------------------+-------+-----------+ | metric | variable | kernel | value | perc_diff | +--------+--------------------+---------------------+-------+-----------+ | gmean | max_frame_duration | EM_default | 251.6 | 0.0% | | gmean | max_frame_duration | EM_modified_runtime | 115.5 | -54.09% | +--------+--------------------+---------------------+-------+-----------+ 4. OS overutilized state percentage (when EAS is not working) +--------------+---------------------+------+------------+------------+ | metric | wa_path | time | total_time | percentage | +--------------+---------------------+------+------------+------------+ | overutilized | EM_default | 1.65 | 253.38 | 0.65 | | overutilized | EM_modified_runtime | 1.4 | 277.5 | 0.51 | +--------------+---------------------+------+------------+------------+ 5. All CPUs (Little+Mid+Big) power values in mW +------------+--------+---------------------+-------+-----------+ | channel | metric | kernel | value | perc_diff | +------------+--------+---------------------+-------+-----------+ | CPU | gmean | EM_default | 142.1 | 0.0% | | CPU | gmean | EM_modified_runtime | 131.8 | -7.27% | +------------+--------+---------------------+-------+-----------+ The time cost to update the EM decreased in this v5 vs v4: big: 5us vs 2us -> 2.6x faster mid: 9us vs 3us -> 3x faster little: 16us vs 16us -> no change We still have to update the inefficiency in the cpufreq framework, thus a bit of overhead will be there. These series is based on linux next tree, tag 'next-20240104', since there are changes from tip/core queueing which touch em_cpu_energy(). Changelog: v6: - renamed 'runtime_table' to 'em_table' (Dietmar, Rafael) - dropped kref increment during allocation (Qais) - renamed em_inc/dec_usage() to em_table_inc/dec() (Qais) - fixed comment description and left old comment block with small adjustment in em_cpu_energy() patch 15/23 (Dietmar) - added platform name which was used for speed-up testing (Dietmar) - changed patch header description keep it small not repeating the in-code comment describing 'cost' in em_cpu_energy() patch 15/23 (Dietmar) - added check and warning in em_cpu_energy() about RCU lock held (Qais, Xuewen) - changed nr_perf_states usage in the patch 7/23 (Dietmar) - changed documentation according to comments (Dietmar) - changed in-code comment in patch 11/23 according to comments (Dietmar) - changed example driver function 'ctx' argument in the documentation (Xuewen) - changed the example driver in documentation, dropped module_exit and added em_free_table() explicit in the update function - fixed comments in various patch headers (Dietmar) - fixed Doxygen comment s/@state/@table patch 4/23 (Dietmar) - added information in the cover letter about: -- optimization in EAS hot code path -- follow-up patch set which adds OPP support and modifies EM for Exynos5 - rebased on 'next-20240104' to avoid collision with other code touching em_cpu_energy() v5 changes are here [4] Regards, Lukasz Luba [1] https://lpc.events/event/16/contributions/1341/attachments/955/1873/Dynamic_Energy_Model_to_handle_leakage_power.pdf [2] https://lpc.events/event/16/contributions/1194/attachments/1114/2139/LPC2022_Energy_model_accuracy.pdf [3] https://www.youtube.com/watch?v=2C-5uikSbtM&list=PL0fKordpLTjKsBOUcZqnzlHShri4YBL1H [4] https://lore.kernel.org/lkml/20231129110853.94344-1-lukasz.luba@arm.com/ [5] https://lore.kernel.org/lkml/20231220110339.1065505-1-lukasz.luba@arm.com/ Lukasz Luba (23): PM: EM: Add missing newline for the message log PM: EM: Refactor em_cpufreq_update_efficiencies() arguments PM: EM: Find first CPU active while updating OPP efficiency PM: EM: Refactor em_pd_get_efficient_state() to be more flexible PM: EM: Refactor a new function em_compute_costs() PM: EM: Check if the get_cost() callback is present in em_compute_costs() PM: EM: Refactor how the EM table is allocated and populated PM: EM: Introduce runtime modifiable table PM: EM: Use runtime modified EM for CPUs energy estimation in EAS PM: EM: Add API for memory allocations for new tables PM: EM: Add API for updating the runtime modifiable EM PM: EM: Add helpers to read under RCU lock the EM table PM: EM: Add performance field to struct em_perf_state and optimize PM: EM: Support late CPUs booting and capacity adjustment PM: EM: Optimize em_cpu_energy() and remove division powercap/dtpm_cpu: Use new Energy Model interface to get table powercap/dtpm_devfreq: Use new Energy Model interface to get table drivers/thermal/cpufreq_cooling: Use new Energy Model interface drivers/thermal/devfreq_cooling: Use new Energy Model interface PM: EM: Change debugfs configuration to use runtime EM table data PM: EM: Remove old table PM: EM: Add em_dev_compute_costs() as API for device drivers Documentation: EM: Update with runtime modification design Documentation/power/energy-model.rst | 183 +++++++++- drivers/powercap/dtpm_cpu.c | 35 +- drivers/powercap/dtpm_devfreq.c | 31 +- drivers/thermal/cpufreq_cooling.c | 40 ++- drivers/thermal/devfreq_cooling.c | 43 ++- include/linux/energy_model.h | 162 +++++---- kernel/power/energy_model.c | 476 +++++++++++++++++++++++---- 7 files changed, 802 insertions(+), 168 deletions(-)