[RESEND][PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
Lukasz Luba
lukasz.luba at arm.com
Wed Jun 24 11:29:47 EDT 2020
On 6/24/20 4:21 PM, Rafael J. Wysocki wrote:
> On Wed, Jun 10, 2020 at 12:12 PM Lukasz Luba <lukasz.luba at arm.com> wrote:
>>
>> Add support for other devices than CPUs. The registration function
>> does not require a valid cpumask pointer and is ready to handle new
>> devices. Some of the internal structures has been reorganized in order to
>> keep consistent view (like removing per_cpu pd pointers).
>>
>> Signed-off-by: Lukasz Luba <lukasz.luba at arm.com>
>> ---
>> Hi all,
>>
>> This is just a small change compared to v8 addressing Rafael's
>> comments an Dan's static analyzes.
>> Here are the changes:
>> - added comment about mutex usage in the unregister function
>> - changed 'dev' into @dev in the kerneldoc comments
>> - removed 'else' statement from em_create_pd() to calm down static analizers
>
> I've applied the series as 5.9 material with patch [4/8] replaced with this one.
>
> Sorry for the delays in handling this!
No worries. Thank you very much!
Regards,
Lukasz
>
> Thanks!
>
>> include/linux/device.h | 5 +
>> include/linux/energy_model.h | 29 ++++-
>> kernel/power/energy_model.c | 244 ++++++++++++++++++++++++-----------
>> 3 files changed, 194 insertions(+), 84 deletions(-)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index ac8e37cd716a..7023d3ea189b 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -13,6 +13,7 @@
>> #define _DEVICE_H_
>>
>> #include <linux/dev_printk.h>
>> +#include <linux/energy_model.h>
>> #include <linux/ioport.h>
>> #include <linux/kobject.h>
>> #include <linux/klist.h>
>> @@ -559,6 +560,10 @@ struct device {
>> struct dev_pm_info power;
>> struct dev_pm_domain *pm_domain;
>>
>> +#ifdef CONFIG_ENERGY_MODEL
>> + struct em_perf_domain *em_pd;
>> +#endif
>> +
>> #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>> struct irq_domain *msi_domain;
>> #endif
>> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
>> index 7076cb22b247..2d4689964029 100644
>> --- a/include/linux/energy_model.h
>> +++ b/include/linux/energy_model.h
>> @@ -12,8 +12,10 @@
>>
>> /**
>> * em_perf_state - Performance state of a performance domain
>> - * @frequency: The CPU frequency in KHz, for consistency with CPUFreq
>> - * @power: The power consumed by 1 CPU at this level, in milli-watts
>> + * @frequency: The frequency in KHz, for consistency with CPUFreq
>> + * @power: The power consumed at this level, in milli-watts (by 1 CPU or
>> + by a registered device). It can be a total power: static and
>> + dynamic.
>> * @cost: The cost coefficient associated with this level, used during
>> * energy calculation. Equal to: power * max_frequency / frequency
>> */
>> @@ -27,12 +29,16 @@ struct em_perf_state {
>> * em_perf_domain - Performance domain
>> * @table: List of performance states, in ascending order
>> * @nr_perf_states: Number of performance states
>> - * @cpus: Cpumask covering the CPUs of the domain
>> + * @cpus: Cpumask covering the CPUs of the domain. It's here
>> + * for performance reasons to avoid potential cache
>> + * misses during energy calculations in the scheduler
>> + * and simplifies allocating/freeing that memory region.
>> *
>> - * A "performance domain" represents a group of CPUs whose performance is
>> - * scaled together. All CPUs of a performance domain must have the same
>> - * micro-architecture. Performance domains often have a 1-to-1 mapping with
>> - * CPUFreq policies.
>> + * In case of CPU device, a "performance domain" represents a group of CPUs
>> + * whose performance is scaled together. All CPUs of a performance domain
>> + * must have the same micro-architecture. Performance domains often have
>> + * a 1-to-1 mapping with CPUFreq policies. In case of other devices the @cpus
>> + * field is unused.
>> */
>> struct em_perf_domain {
>> struct em_perf_state *table;
>> @@ -71,10 +77,12 @@ struct em_data_callback {
>> #define EM_DATA_CB(_active_power_cb) { .active_power = &_active_power_cb }
>>
>> struct em_perf_domain *em_cpu_get(int cpu);
>> +struct em_perf_domain *em_pd_get(struct device *dev);
>> int em_register_perf_domain(cpumask_t *span, unsigned int nr_states,
>> struct em_data_callback *cb);
>> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>> struct em_data_callback *cb, cpumask_t *span);
>> +void em_dev_unregister_perf_domain(struct device *dev);
>>
>> /**
>> * em_pd_energy() - Estimates the energy consumed by the CPUs of a perf. domain
>> @@ -184,10 +192,17 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>> {
>> return -EINVAL;
>> }
>> +static inline void em_dev_unregister_perf_domain(struct device *dev)
>> +{
>> +}
>> static inline struct em_perf_domain *em_cpu_get(int cpu)
>> {
>> return NULL;
>> }
>> +static inline struct em_perf_domain *em_pd_get(struct device *dev)
>> +{
>> + return NULL;
>> +}
>> static inline unsigned long em_pd_energy(struct em_perf_domain *pd,
>> unsigned long max_util, unsigned long sum_util)
>> {
>> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
>> index 5b8a1566526a..32d76e78f992 100644
>> --- a/kernel/power/energy_model.c
>> +++ b/kernel/power/energy_model.c
>> @@ -1,9 +1,10 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> - * Energy Model of CPUs
>> + * Energy Model of devices
>> *
>> - * Copyright (c) 2018, Arm ltd.
>> + * Copyright (c) 2018-2020, Arm ltd.
>> * Written by: Quentin Perret, Arm ltd.
>> + * Improvements provided by: Lukasz Luba, Arm ltd.
>> */
>>
>> #define pr_fmt(fmt) "energy_model: " fmt
>> @@ -15,15 +16,17 @@
>> #include <linux/sched/topology.h>
>> #include <linux/slab.h>
>>
>> -/* Mapping of each CPU to the performance domain to which it belongs. */
>> -static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
>> -
>> /*
>> * Mutex serializing the registrations of performance domains and letting
>> * callbacks defined by drivers sleep.
>> */
>> static DEFINE_MUTEX(em_pd_mutex);
>>
>> +static bool _is_cpu_device(struct device *dev)
>> +{
>> + return (dev->bus == &cpu_subsys);
>> +}
>> +
>> #ifdef CONFIG_DEBUG_FS
>> static struct dentry *rootdir;
>>
>> @@ -49,22 +52,30 @@ static int em_debug_cpus_show(struct seq_file *s, void *unused)
>> }
>> DEFINE_SHOW_ATTRIBUTE(em_debug_cpus);
>>
>> -static void em_debug_create_pd(struct em_perf_domain *pd, int cpu)
>> +static void em_debug_create_pd(struct device *dev)
>> {
>> struct dentry *d;
>> - char name[8];
>> int i;
>>
>> - snprintf(name, sizeof(name), "pd%d", cpu);
>> -
>> /* Create the directory of the performance domain */
>> - d = debugfs_create_dir(name, rootdir);
>> + d = debugfs_create_dir(dev_name(dev), rootdir);
>>
>> - debugfs_create_file("cpus", 0444, d, pd->cpus, &em_debug_cpus_fops);
>> + if (_is_cpu_device(dev))
>> + debugfs_create_file("cpus", 0444, d, dev->em_pd->cpus,
>> + &em_debug_cpus_fops);
>>
>> /* Create a sub-directory for each performance state */
>> - for (i = 0; i < pd->nr_perf_states; i++)
>> - em_debug_create_ps(&pd->table[i], d);
>> + for (i = 0; i < dev->em_pd->nr_perf_states; i++)
>> + em_debug_create_ps(&dev->em_pd->table[i], d);
>> +
>> +}
>> +
>> +static void em_debug_remove_pd(struct device *dev)
>> +{
>> + struct dentry *debug_dir;
>> +
>> + debug_dir = debugfs_lookup(dev_name(dev), rootdir);
>> + debugfs_remove_recursive(debug_dir);
>> }
>>
>> static int __init em_debug_init(void)
>> @@ -76,40 +87,34 @@ static int __init em_debug_init(void)
>> }
>> core_initcall(em_debug_init);
>> #else /* CONFIG_DEBUG_FS */
>> -static void em_debug_create_pd(struct em_perf_domain *pd, int cpu) {}
>> +static void em_debug_create_pd(struct device *dev) {}
>> +static void em_debug_remove_pd(struct device *dev) {}
>> #endif
>> -static struct em_perf_domain *
>> -em_create_pd(struct device *dev, int nr_states, struct em_data_callback *cb,
>> - cpumask_t *span)
>> +
>> +static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
>> + int nr_states, struct em_data_callback *cb)
>> {
>> unsigned long opp_eff, prev_opp_eff = ULONG_MAX;
>> unsigned long power, freq, prev_freq = 0;
>> - int i, ret, cpu = cpumask_first(span);
>> struct em_perf_state *table;
>> - struct em_perf_domain *pd;
>> + int i, ret;
>> u64 fmax;
>>
>> - if (!cb->active_power)
>> - return NULL;
>> -
>> - pd = kzalloc(sizeof(*pd) + cpumask_size(), GFP_KERNEL);
>> - if (!pd)
>> - return NULL;
>> -
>> table = kcalloc(nr_states, sizeof(*table), GFP_KERNEL);
>> if (!table)
>> - goto free_pd;
>> + return -ENOMEM;
>>
>> /* Build the list of performance states for this performance domain */
>> for (i = 0, freq = 0; i < nr_states; i++, freq++) {
>> /*
>> * active_power() is a driver callback which ceils 'freq' to
>> - * lowest performance state of 'cpu' above 'freq' and updates
>> + * lowest performance state of 'dev' above 'freq' and updates
>> * 'power' and 'freq' accordingly.
>> */
>> ret = cb->active_power(&power, &freq, dev);
>> if (ret) {
>> - pr_err("pd%d: invalid perf. state: %d\n", cpu, ret);
>> + dev_err(dev, "EM: invalid perf. state: %d\n",
>> + ret);
>> goto free_ps_table;
>> }
>>
>> @@ -118,7 +123,8 @@ em_create_pd(struct device *dev, int nr_states, struct em_data_callback *cb,
>> * higher performance states.
>> */
>> if (freq <= prev_freq) {
>> - pr_err("pd%d: non-increasing freq: %lu\n", cpu, freq);
>> + dev_err(dev, "EM: non-increasing freq: %lu\n",
>> + freq);
>> goto free_ps_table;
>> }
>>
>> @@ -127,7 +133,8 @@ em_create_pd(struct device *dev, int nr_states, struct em_data_callback *cb,
>> * positive, in milli-watts and to fit into 16 bits.
>> */
>> if (!power || power > EM_MAX_POWER) {
>> - pr_err("pd%d: invalid power: %lu\n", cpu, power);
>> + dev_err(dev, "EM: invalid power: %lu\n",
>> + power);
>> goto free_ps_table;
>> }
>>
>> @@ -142,8 +149,8 @@ em_create_pd(struct device *dev, int nr_states, struct em_data_callback *cb,
>> */
>> opp_eff = freq / power;
>> if (opp_eff >= prev_opp_eff)
>> - pr_warn("pd%d: hertz/watts ratio non-monotonically decreasing: em_perf_state %d >= em_perf_state%d\n",
>> - cpu, i, i - 1);
>> + dev_dbg(dev, "EM: hertz/watts ratio non-monotonically decreasing: em_perf_state %d >= em_perf_state%d\n",
>> + i, i - 1);
>> prev_opp_eff = opp_eff;
>> }
>>
>> @@ -156,30 +163,82 @@ em_create_pd(struct device *dev, int nr_states, struct em_data_callback *cb,
>>
>> pd->table = table;
>> pd->nr_perf_states = nr_states;
>> - cpumask_copy(to_cpumask(pd->cpus), span);
>>
>> - em_debug_create_pd(pd, cpu);
>> -
>> - return pd;
>> + return 0;
>>
>> free_ps_table:
>> kfree(table);
>> -free_pd:
>> - kfree(pd);
>> + return -EINVAL;
>> +}
>> +
>> +static int em_create_pd(struct device *dev, int nr_states,
>> + struct em_data_callback *cb, cpumask_t *cpus)
>> +{
>> + struct em_perf_domain *pd;
>> + struct device *cpu_dev;
>> + int cpu, ret;
>> +
>> + if (_is_cpu_device(dev)) {
>> + pd = kzalloc(sizeof(*pd) + cpumask_size(), GFP_KERNEL);
>> + if (!pd)
>> + return -ENOMEM;
>> +
>> + cpumask_copy(em_span_cpus(pd), cpus);
>> + } else {
>> + pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>> + if (!pd)
>> + return -ENOMEM;
>> + }
>> +
>> + ret = em_create_perf_table(dev, pd, nr_states, cb);
>> + if (ret) {
>> + kfree(pd);
>> + return ret;
>> + }
>> +
>> + if (_is_cpu_device(dev))
>> + for_each_cpu(cpu, cpus) {
>> + cpu_dev = get_cpu_device(cpu);
>> + cpu_dev->em_pd = pd;
>> + }
>> +
>> + dev->em_pd = pd;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * em_pd_get() - Return the performance domain for a device
>> + * @dev : Device to find the performance domain for
>> + *
>> + * Returns the performance domain to which @dev belongs, or NULL if it doesn't
>> + * exist.
>> + */
>> +struct em_perf_domain *em_pd_get(struct device *dev)
>> +{
>> + if (IS_ERR_OR_NULL(dev))
>> + return NULL;
>>
>> - return NULL;
>> + return dev->em_pd;
>> }
>> +EXPORT_SYMBOL_GPL(em_pd_get);
>>
>> /**
>> * em_cpu_get() - Return the performance domain for a CPU
>> * @cpu : CPU to find the performance domain for
>> *
>> - * Return: the performance domain to which 'cpu' belongs, or NULL if it doesn't
>> + * Returns the performance domain to which @cpu belongs, or NULL if it doesn't
>> * exist.
>> */
>> struct em_perf_domain *em_cpu_get(int cpu)
>> {
>> - return READ_ONCE(per_cpu(em_data, cpu));
>> + struct device *cpu_dev;
>> +
>> + cpu_dev = get_cpu_device(cpu);
>> + if (!cpu_dev)
>> + return NULL;
>> +
>> + return em_pd_get(cpu_dev);
>> }
>> EXPORT_SYMBOL_GPL(em_cpu_get);
>>
>> @@ -188,7 +247,7 @@ EXPORT_SYMBOL_GPL(em_cpu_get);
>> * @dev : Device for which the EM is to register
>> * @nr_states : Number of performance states to register
>> * @cb : Callback functions providing the data of the Energy Model
>> - * @span : Pointer to cpumask_t, which in case of a CPU device is
>> + * @cpus : Pointer to cpumask_t, which in case of a CPU device is
>> * obligatory. It can be taken from i.e. 'policy->cpus'. For other
>> * type of devices this should be set to NULL.
>> *
>> @@ -201,13 +260,12 @@ EXPORT_SYMBOL_GPL(em_cpu_get);
>> * Return 0 on success
>> */
>> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>> - struct em_data_callback *cb, cpumask_t *span)
>> + struct em_data_callback *cb, cpumask_t *cpus)
>> {
>> unsigned long cap, prev_cap = 0;
>> - struct em_perf_domain *pd;
>> - int cpu, ret = 0;
>> + int cpu, ret;
>>
>> - if (!dev || !span || !nr_states || !cb)
>> + if (!dev || !nr_states || !cb)
>> return -EINVAL;
>>
>> /*
>> @@ -216,47 +274,50 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>> */
>> mutex_lock(&em_pd_mutex);
>>
>> - for_each_cpu(cpu, span) {
>> - /* Make sure we don't register again an existing domain. */
>> - if (READ_ONCE(per_cpu(em_data, cpu))) {
>> - ret = -EEXIST;
>> - goto unlock;
>> - }
>> + if (dev->em_pd) {
>> + ret = -EEXIST;
>> + goto unlock;
>> + }
>>
>> - /*
>> - * All CPUs of a domain must have the same micro-architecture
>> - * since they all share the same table.
>> - */
>> - cap = arch_scale_cpu_capacity(cpu);
>> - if (prev_cap && prev_cap != cap) {
>> - pr_err("CPUs of %*pbl must have the same capacity\n",
>> - cpumask_pr_args(span));
>> + if (_is_cpu_device(dev)) {
>> + if (!cpus) {
>> + dev_err(dev, "EM: invalid CPU mask\n");
>> ret = -EINVAL;
>> goto unlock;
>> }
>> - prev_cap = cap;
>> +
>> + for_each_cpu(cpu, cpus) {
>> + if (em_cpu_get(cpu)) {
>> + dev_err(dev, "EM: exists for CPU%d\n", cpu);
>> + ret = -EEXIST;
>> + goto unlock;
>> + }
>> + /*
>> + * All CPUs of a domain must have the same
>> + * micro-architecture since they all share the same
>> + * table.
>> + */
>> + cap = arch_scale_cpu_capacity(cpu);
>> + if (prev_cap && prev_cap != cap) {
>> + dev_err(dev, "EM: CPUs of %*pbl must have the same capacity\n",
>> + cpumask_pr_args(cpus));
>> +
>> + ret = -EINVAL;
>> + goto unlock;
>> + }
>> + prev_cap = cap;
>> + }
>> }
>>
>> - /* Create the performance domain and add it to the Energy Model. */
>> - pd = em_create_pd(dev, nr_states, cb, span);
>> - if (!pd) {
>> - ret = -EINVAL;
>> + ret = em_create_pd(dev, nr_states, cb, cpus);
>> + if (ret)
>> goto unlock;
>> - }
>>
>> - for_each_cpu(cpu, span) {
>> - /*
>> - * The per-cpu array can be read concurrently from em_cpu_get().
>> - * The barrier enforces the ordering needed to make sure readers
>> - * can only access well formed em_perf_domain structs.
>> - */
>> - smp_store_release(per_cpu_ptr(&em_data, cpu), pd);
>> - }
>> + em_debug_create_pd(dev);
>> + dev_info(dev, "EM: created perf domain\n");
>>
>> - pr_debug("Created perf domain %*pbl\n", cpumask_pr_args(span));
>> unlock:
>> mutex_unlock(&em_pd_mutex);
>> -
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(em_dev_register_perf_domain);
>> @@ -285,3 +346,32 @@ int em_register_perf_domain(cpumask_t *span, unsigned int nr_states,
>> return em_dev_register_perf_domain(cpu_dev, nr_states, cb, span);
>> }
>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
>> +
>> +/**
>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
>> + * @dev : Device for which the EM is registered
>> + *
>> + * Unregister the EM for the specified @dev (but not a CPU device).
>> + */
>> +void em_dev_unregister_perf_domain(struct device *dev)
>> +{
>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
>> + return;
>> +
>> + if (_is_cpu_device(dev))
>> + return;
>> +
>> + /*
>> + * The mutex separates all register/unregister requests and protects
>> + * from potential clean-up/setup issues in the debugfs directories.
>> + * The debugfs directory name is the same as device's name.
>> + */
>> + mutex_lock(&em_pd_mutex);
>> + em_debug_remove_pd(dev);
>> +
>> + kfree(dev->em_pd->table);
>> + kfree(dev->em_pd);
>> + dev->em_pd = NULL;
>> + mutex_unlock(&em_pd_mutex);
>> +}
>> +EXPORT_SYMBOL_GPL(em_dev_unregister_perf_domain);
>> --
>> 2.17.1
>>
More information about the linux-arm-kernel
mailing list