2023-05-12 10:07:51

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 00/17] Introduce runtime modifiable Energy Model

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's part of feature set know as Dynamic Energy Model. It has been presented
and discussed recently at OSPM2023 [3]. This patch set implements the 1st
improvement for the EM.

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 or EM can be adjusted and reflect the fact of increased
power usage. That power increase is due to a factor called 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'.

Some design details:
The internal mechanisms for the memory allocation are handled internally in the
EM. Kernel modules can just call the new API to update the EM data and the
new memory would be provided and owned by the EM. The EM memory is used by
EAS, which impacts those design decisions. The EM writers are protected by
a mutex. This new runtime modified EM table is protected using RCU mechanism,
which fits the current EAS hot path (which already uses RCU read lock).
The unregister API handles only non-CPU (e.g. GPU, ISP) devices and uses the
same mutex as EM modifiers to make sure the memory is safely freed.

More detailed explanation and background can be found in presentations
during LPC2022 [1][2] or in the documentation patches.

Changelog:
v2:
- solved build warning of unused variable in patch 13/17 when EM is
not compiled in, e.g. on Intel platform for this cpufreq_cooling
- re-based on top of v6.4-rc1
v1:
- implementation can be found 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/[email protected]/

Lukasz Luba (17):
PM: EM: Refactor em_cpufreq_update_efficiencies() arguments
PM: EM: Find first CPU online while updating OPP efficiency
PM: EM: Refactor em_pd_get_efficient_state() to be more flexible
PM: EM: Create a new function em_compute_costs()
trace: energy_model: Add trace event for EM runtime modifications
PM: EM: Add update_power() callback for runtime modifications
PM: EM: Check if the get_cost() callback is present in
em_compute_costs()
PM: EM: Introduce runtime modifiable table
PM: EM: Add RCU mechanism which safely cleans the old data
PM: EM: Add runtime update interface to modify EM power
PM: EM: Use runtime modified EM for CPUs energy estimation in EAS
PM: EM: Add argument to get_cost() for runtime modification
PM: EM: Refactor struct em_perf_domain and add default_table
Documentation: EM: Add a new section about the design
Documentation: EM: Add a runtime modifiable EM design description
Documentation: EM: Add example with driver modifying the EM
Documentation: EM: Describe the API of runtime modifications

Documentation/power/energy-model.rst | 134 +++++++++++-
drivers/cpufreq/cppc_cpufreq.c | 2 +-
drivers/powercap/dtpm_cpu.c | 27 ++-
drivers/powercap/dtpm_devfreq.c | 23 +-
drivers/thermal/cpufreq_cooling.c | 23 +-
drivers/thermal/devfreq_cooling.c | 23 +-
include/linux/energy_model.h | 93 ++++++--
include/trace/events/energy_model.h | 46 ++++
kernel/power/energy_model.c | 305 +++++++++++++++++++++++----
9 files changed, 580 insertions(+), 96 deletions(-)
create mode 100644 include/trace/events/energy_model.h

--
2.25.1



2023-05-12 10:08:08

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 10/17] PM: EM: Add runtime update interface to modify EM power

Add an interface which allows to modify EM power data at runtime.
The new power information is populated by the provided callback, which
is called for each performance state. The CPU frequencies' efficiency is
re-calculated since that might be affected as well. The old EM memory
is going to be freed later using RCU mechanism.

Signed-off-by: Lukasz Luba <[email protected]>
---
include/linux/energy_model.h | 8 +++
kernel/power/energy_model.c | 109 +++++++++++++++++++++++++++++++++++
2 files changed, 117 insertions(+)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index a616006a8130..e1772aa6c843 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -202,6 +202,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_data_callback *cb,
+ void *priv);
int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
struct em_data_callback *cb, cpumask_t *span,
bool microwatts);
@@ -382,6 +384,12 @@ static inline int em_pd_nr_perf_states(struct em_perf_domain *pd)
{
return 0;
}
+static inline
+int em_dev_update_perf_domain(struct device *dev, struct em_data_callback *cb,
+ void *priv)
+{
+ return -EINVAL;
+}
#endif

#endif
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index bc9b7dec0763..b5675dda00e1 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -202,6 +202,101 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table,
return 0;
}

+/**
+ * em_dev_update_perf_domain() - Update run-time EM table for a device
+ * @dev : Device for which the EM is to be updated
+ * @cb : Callback function providing the power data for the EM
+ * @priv : Pointer to private data useful for passing context
+ * which might be required while calling @cb
+ *
+ * Update EM run-time modifiable table for a @dev using the callback
+ * defined in @cb. The EM new power values are then used for calculating
+ * the em_perf_state::cost for associated performance state.
+ *
+ * This function uses mutex to serialize writers, so it must not be called
+ * from non-sleeping context.
+ *
+ * Return 0 on success or a proper error in case of failure.
+ */
+int em_dev_update_perf_domain(struct device *dev, struct em_data_callback *cb,
+ void *priv)
+{
+ struct em_perf_table *runtime_table;
+ unsigned long power, freq;
+ struct em_perf_domain *pd;
+ int ret, i;
+
+ if (!cb || !cb->update_power)
+ return -EINVAL;
+
+ /*
+ * The lock serializes update and unregister code paths. When the
+ * EM has been unregistered in the meantime, we should capture that
+ * when entering this critical section. It also makes sure that
+ * two concurrent updates will be serialized.
+ */
+ mutex_lock(&em_pd_mutex);
+
+ if (!dev || !dev->em_pd) {
+ ret = -EINVAL;
+ goto unlock_em;
+ }
+
+ pd = dev->em_pd;
+
+ runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
+ if (!runtime_table) {
+ ret = -ENOMEM;
+ goto unlock_em;
+ }
+
+ runtime_table->state = kcalloc(pd->nr_perf_states,
+ sizeof(struct em_perf_state),
+ GFP_KERNEL);
+ if (!runtime_table->state) {
+ ret = -ENOMEM;
+ goto free_runtime_table;
+ }
+
+ /* Populate runtime table with updated values using driver callback */
+ for (i = 0; i < pd->nr_perf_states; i++) {
+ freq = pd->table[i].frequency;
+ runtime_table->state[i].frequency = freq;
+
+ /*
+ * Call driver callback to get a new power value for
+ * a given frequency.
+ */
+ ret = cb->update_power(dev, freq, &power, priv);
+ if (ret) {
+ dev_dbg(dev, "EM: run-time update error: %d\n", ret);
+ goto free_runtime_state_table;
+ }
+
+ runtime_table->state[i].power = power;
+ }
+
+ ret = em_compute_costs(dev, runtime_table->state, cb,
+ pd->nr_perf_states, pd->flags);
+ if (ret)
+ goto free_runtime_state_table;
+
+ em_perf_runtime_table_set(dev, runtime_table);
+
+ mutex_unlock(&em_pd_mutex);
+ return 0;
+
+free_runtime_state_table:
+ kfree(runtime_table->state);
+free_runtime_table:
+ kfree(runtime_table);
+unlock_em:
+ mutex_unlock(&em_pd_mutex);
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(em_dev_update_perf_domain);
+
static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
int nr_states, struct em_data_callback *cb,
unsigned long flags)
@@ -521,6 +616,8 @@ void em_dev_unregister_perf_domain(struct device *dev)
* 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.
+ * The lock also protects the updater of the runtime modifiable
+ * EM and this remover.
*/
mutex_lock(&em_pd_mutex);

@@ -528,9 +625,21 @@ void em_dev_unregister_perf_domain(struct device *dev)

tmp = pd->runtime_table;

+ /*
+ * Safely destroy runtime modifiable EM. By using the call
+ * synchronize_rcu() we make sure we don't progress till last user
+ * finished the RCU section and our update got applied.
+ */
rcu_assign_pointer(pd->runtime_table, NULL);
synchronize_rcu();

+ /*
+ * After the sync no updates will be in-flight, so free the old
+ * memory.
+ */
+ if (tmp->state != pd->table)
+ kfree(tmp->state);
+
kfree(tmp);

kfree(dev->em_pd->table);
--
2.25.1


2023-05-12 10:08:21

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 08/17] PM: EM: Introduce runtime modifiable table

This patch introduces the new feature: modifiable EM perf_state table.
The new runtime table would be populated with a new power data to better
reflect the actual power. The power can vary over time e.g. due to the
SoC temperature change. Higher temperature can increase power values.
For longer running scenarios, such as game or camera, when also other
devices are used (e.g. GPU, ISP) the CPU power can change. The new
EM framework is able to addresses this issue and change the data
at runtime safely. The runtime modifiable EM data is used by the Energy
Aware Scheduler (EAS) for the task placement.

Signed-off-by: Lukasz Luba <[email protected]>
---
include/linux/energy_model.h | 13 +++++++++++++
kernel/power/energy_model.c | 24 ++++++++++++++++++++++++
2 files changed, 37 insertions(+)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index cc2bf607191e..a616006a8130 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -36,9 +36,21 @@ struct em_perf_state {
*/
#define EM_PERF_STATE_INEFFICIENT BIT(0)

+/**
+ * struct em_perf_table - Performance states table, which can be
+ * runtime modifiable and protected with RCU
+ * @state: List of performance states, in ascending order
+ * @rcu: RCU used for safe access and destruction
+ */
+struct em_perf_table {
+ struct em_perf_state *state;
+ struct rcu_head rcu;
+};
+
/**
* struct em_perf_domain - Performance domain
* @table: List of performance states, in ascending order
+ * @runtime_table: Pointer to the runtime modified em_perf_table
* @nr_perf_states: Number of performance states
* @flags: See "em_perf_domain flags"
* @cpus: Cpumask covering the CPUs of the domain. It's here
@@ -54,6 +66,7 @@ struct em_perf_state {
*/
struct em_perf_domain {
struct em_perf_state *table;
+ struct em_perf_table __rcu *runtime_table;
int nr_perf_states;
unsigned long flags;
unsigned long cpus[];
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 8866d217714e..39d47028ef3d 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -213,6 +213,7 @@ static int em_create_pd(struct device *dev, int nr_states,
struct em_data_callback *cb, cpumask_t *cpus,
unsigned long flags)
{
+ struct em_perf_table *runtime_table;
struct em_perf_domain *pd;
struct device *cpu_dev;
int cpu, ret, num_cpus;
@@ -237,12 +238,23 @@ static int em_create_pd(struct device *dev, int nr_states,
return -ENOMEM;
}

+ runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
+ if (!runtime_table) {
+ kfree(pd);
+ return -ENOMEM;
+ }
+
ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
if (ret) {
kfree(pd);
+ kfree(runtime_table);
return ret;
}

+ /* Re-use temporally (till 1st modification) the memory */
+ runtime_table->state = pd->table;
+ rcu_assign_pointer(pd->runtime_table, runtime_table);
+
if (_is_cpu_device(dev))
for_each_cpu(cpu, cpus) {
cpu_dev = get_cpu_device(cpu);
@@ -438,20 +450,32 @@ EXPORT_SYMBOL_GPL(em_dev_register_perf_domain);
*/
void em_dev_unregister_perf_domain(struct device *dev)
{
+ struct em_perf_domain *pd;
+ struct em_perf_table *tmp;
+
if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
return;

if (_is_cpu_device(dev))
return;

+ pd = dev->em_pd;
/*
* 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);

+ tmp = pd->runtime_table;
+
+ rcu_assign_pointer(pd->runtime_table, NULL);
+ synchronize_rcu();
+
+ kfree(tmp);
+
kfree(dev->em_pd->table);
kfree(dev->em_pd);
dev->em_pd = NULL;
--
2.25.1


2023-05-12 10:08:22

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 12/17] PM: EM: Add argument to get_cost() for runtime modification

The Energy Model (EM) supports runtime modifications. Let also the
artificial EM use this new feature and allow to update the 'cost' values
at runtime. When the artificial EM is used there is a need to provide
two callbacks: get_cost() and update_power(), not only the last one.

Update also CPPC driver code, since the new argument is needed there
to compile properly and register EM.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/cpufreq/cppc_cpufreq.c | 2 +-
include/linux/energy_model.h | 7 ++++++-
kernel/power/energy_model.c | 9 +++++----
3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 022e3555407c..f5353c10552b 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -574,7 +574,7 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
}

static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
- unsigned long *cost)
+ unsigned long *cost, void *priv)
{
unsigned long perf_step, perf_prev;
struct cppc_perf_caps *perf_caps;
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index 8e3fa2b6bf28..b8506df9af2d 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -162,6 +162,8 @@ struct em_data_callback {
* @freq : Frequency at the performance state in kHz
* @cost : The cost value for the performance state
* (modified)
+ * @priv : Pointer to private data useful for tracking context
+ * during run-time modifications of EM.
*
* In case of CPUs, the cost is the one of a single CPU in the domain.
* It is expected to fit in the [0, EM_MAX_POWER] range due to internal
@@ -170,7 +172,7 @@ struct em_data_callback {
* Return 0 on success, or appropriate error value in case of failure.
*/
int (*get_cost)(struct device *dev, unsigned long freq,
- unsigned long *cost);
+ unsigned long *cost, void *priv);

/**
* update_power() - Provide new power at the given performance state of
@@ -199,6 +201,9 @@ struct em_data_callback {
#define EM_DATA_CB(_active_power_cb) \
EM_ADV_DATA_CB(_active_power_cb, NULL)
#define EM_UPDATE_CB(_update_power_cb) { .update_power = &_update_power_cb }
+#define EM_ADV_UPDATE_CB(_update_power_cb, _cost_cb) \
+ { .update_power = &_update_power_cb, \
+ .get_cost = _cost_cb }

struct em_perf_domain *em_cpu_get(int cpu);
struct em_perf_domain *em_pd_get(struct device *dev);
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index b5675dda00e1..456d9f2b4370 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -165,7 +165,7 @@ static void em_perf_runtime_table_set(struct device *dev,

static int em_compute_costs(struct device *dev, struct em_perf_state *table,
struct em_data_callback *cb, int nr_states,
- unsigned long flags)
+ unsigned long flags, void *priv)
{
unsigned long prev_cost = ULONG_MAX;
u64 fmax;
@@ -177,7 +177,8 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table,
unsigned long power_res, cost;

if (flags & EM_PERF_DOMAIN_ARTIFICIAL && cb->get_cost) {
- ret = cb->get_cost(dev, table[i].frequency, &cost);
+ ret = cb->get_cost(dev, table[i].frequency, &cost,
+ priv);
if (ret || !cost || cost > EM_MAX_POWER) {
dev_err(dev, "EM: invalid cost %lu %d\n",
cost, ret);
@@ -277,7 +278,7 @@ int em_dev_update_perf_domain(struct device *dev, struct em_data_callback *cb,
}

ret = em_compute_costs(dev, runtime_table->state, cb,
- pd->nr_perf_states, pd->flags);
+ pd->nr_perf_states, pd->flags, priv);
if (ret)
goto free_runtime_state_table;

@@ -347,7 +348,7 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
table[i].frequency = prev_freq = freq;
}

- ret = em_compute_costs(dev, table, cb, nr_states, flags);
+ ret = em_compute_costs(dev, table, cb, nr_states, flags, NULL);
if (ret)
goto free_ps_table;

--
2.25.1


2023-05-12 10:08:25

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 15/17] Documentation: EM: Add a runtime modifiable EM design description

Document the new runtime modifiable EM design and how it can be used.
Change the last section layout and allow to provide another example
how to use this new API in a driver code.

Signed-off-by: Lukasz Luba <[email protected]>
---
Documentation/power/energy-model.rst | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/Documentation/power/energy-model.rst b/Documentation/power/energy-model.rst
index e97c7f18d8bd..64c2462dc9a6 100644
--- a/Documentation/power/energy-model.rst
+++ b/Documentation/power/energy-model.rst
@@ -89,6 +89,26 @@ due to the workload instruction mix and data set is not modeled.
Also static power which can change during runtime due to variation of SOC
temperature is not modeled in EM.

+2.2 Runtime modifiable EM
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+To better reflect power variation due to static power (leakage) the EM
+supports runtime modifications of the power values. The mechanism relies on
+RCU to free the modifiable EM perf_state table memory. Its user, the task
+scheduler, also uses RCU to access this memory. The EM framework is
+responsible for allocating the new memory for the modifiable EM perf_state
+table. The old memory is freed automatically using RCU callback mechanism.
+This design decision is made based on task scheduler using that data and
+to prevent wrong usage of kernel modules if they would be responsible for the
+memory management.
+The drivers which want to modify the EM values are protected from concurrent
+access using a mutex. Therefore, the drivers must use sleeping context when
+they want to modify the EM. The runtime modifiable EM might also be used for
+better reflecting real workload scenarios, e.g. when they pop-up on the screen
+and will run for longer period, such as: games, video recoding or playing,
+video calls, etc. It is up to the platform engineers to experiment and choose
+the right approach for their device.
+

3. Core APIs
------------
--
2.25.1


2023-05-12 10:09:00

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 13/17] PM: EM: Refactor struct em_perf_domain and add default_table

The Energy Model support runtime modifications. Refactor old
implementation which accessed struct em_perf_state and introduce
em_perf_domain::default_table to clean up the design. This new field
is better aligned with em_perf_domain::runtime_table and helps to
distinguish them better.

Update all drivers or frameworks which used the old field:
em_perf_domain::table and now should use em_perf_domain::default_table.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/powercap/dtpm_cpu.c | 27 ++++++++++++++++++--------
drivers/powercap/dtpm_devfreq.c | 23 +++++++++++++++-------
drivers/thermal/cpufreq_cooling.c | 23 ++++++++++++++--------
drivers/thermal/devfreq_cooling.c | 23 ++++++++++++++++------
include/linux/energy_model.h | 4 ++--
kernel/power/energy_model.c | 32 ++++++++++++++++++++++---------
6 files changed, 92 insertions(+), 40 deletions(-)

diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
index 2ff7717530bf..743a0ac8ecdf 100644
--- a/drivers/powercap/dtpm_cpu.c
+++ b/drivers/powercap/dtpm_cpu.c
@@ -43,6 +43,7 @@ static u64 set_pd_power_limit(struct dtpm *dtpm, u64 power_limit)
{
struct dtpm_cpu *dtpm_cpu = to_dtpm_cpu(dtpm);
struct em_perf_domain *pd = em_cpu_get(dtpm_cpu->cpu);
+ struct em_perf_state *table;
struct cpumask cpus;
unsigned long freq;
u64 power;
@@ -51,19 +52,21 @@ static u64 set_pd_power_limit(struct dtpm *dtpm, u64 power_limit)
cpumask_and(&cpus, cpu_online_mask, to_cpumask(pd->cpus));
nr_cpus = cpumask_weight(&cpus);

+ table = pd->default_table->state;
+
for (i = 0; i < pd->nr_perf_states; i++) {

- power = pd->table[i].power * nr_cpus;
+ power = table[i].power * nr_cpus;

if (power > power_limit)
break;
}

- freq = pd->table[i - 1].frequency;
+ freq = table[i - 1].frequency;

freq_qos_update_request(&dtpm_cpu->qos_req, freq);

- power_limit = pd->table[i - 1].power * nr_cpus;
+ power_limit = table[i - 1].power * nr_cpus;

return power_limit;
}
@@ -88,12 +91,14 @@ static u64 scale_pd_power_uw(struct cpumask *pd_mask, u64 power)
static u64 get_pd_power_uw(struct dtpm *dtpm)
{
struct dtpm_cpu *dtpm_cpu = to_dtpm_cpu(dtpm);
+ struct em_perf_state *table;
struct em_perf_domain *pd;
struct cpumask *pd_mask;
unsigned long freq;
int i;

pd = em_cpu_get(dtpm_cpu->cpu);
+ table = pd->default_table->state;

pd_mask = em_span_cpus(pd);

@@ -101,10 +106,10 @@ static u64 get_pd_power_uw(struct dtpm *dtpm)

for (i = 0; i < pd->nr_perf_states; i++) {

- if (pd->table[i].frequency < freq)
+ if (table[i].frequency < freq)
continue;

- return scale_pd_power_uw(pd_mask, pd->table[i].power *
+ return scale_pd_power_uw(pd_mask, table[i].power *
MICROWATT_PER_MILLIWATT);
}

@@ -115,17 +120,20 @@ static int update_pd_power_uw(struct dtpm *dtpm)
{
struct dtpm_cpu *dtpm_cpu = to_dtpm_cpu(dtpm);
struct em_perf_domain *em = em_cpu_get(dtpm_cpu->cpu);
+ struct em_perf_state *table;
struct cpumask cpus;
int nr_cpus;

cpumask_and(&cpus, cpu_online_mask, to_cpumask(em->cpus));
nr_cpus = cpumask_weight(&cpus);

- dtpm->power_min = em->table[0].power;
+ table = em->default_table->state;
+
+ dtpm->power_min = table[0].power;
dtpm->power_min *= MICROWATT_PER_MILLIWATT;
dtpm->power_min *= nr_cpus;

- dtpm->power_max = em->table[em->nr_perf_states - 1].power;
+ dtpm->power_max = table[em->nr_perf_states - 1].power;
dtpm->power_max *= MICROWATT_PER_MILLIWATT;
dtpm->power_max *= nr_cpus;

@@ -182,6 +190,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
{
struct dtpm_cpu *dtpm_cpu;
struct cpufreq_policy *policy;
+ struct em_perf_state *table;
struct em_perf_domain *pd;
char name[CPUFREQ_NAME_LEN];
int ret = -ENOMEM;
@@ -198,6 +207,8 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
if (!pd || em_is_artificial(pd))
return -EINVAL;

+ table = pd->default_table->state;
+
dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL);
if (!dtpm_cpu)
return -ENOMEM;
@@ -216,7 +227,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)

ret = freq_qos_add_request(&policy->constraints,
&dtpm_cpu->qos_req, FREQ_QOS_MAX,
- pd->table[pd->nr_perf_states - 1].frequency);
+ table[pd->nr_perf_states - 1].frequency);
if (ret)
goto out_dtpm_unregister;

diff --git a/drivers/powercap/dtpm_devfreq.c b/drivers/powercap/dtpm_devfreq.c
index 91276761a31d..6ef0f2b4a683 100644
--- a/drivers/powercap/dtpm_devfreq.c
+++ b/drivers/powercap/dtpm_devfreq.c
@@ -37,11 +37,14 @@ static int update_pd_power_uw(struct dtpm *dtpm)
struct devfreq *devfreq = dtpm_devfreq->devfreq;
struct device *dev = devfreq->dev.parent;
struct em_perf_domain *pd = em_pd_get(dev);
+ struct em_perf_state *table;

- dtpm->power_min = pd->table[0].power;
+ table = pd->default_table->state;
+
+ dtpm->power_min = table[0].power;
dtpm->power_min *= MICROWATT_PER_MILLIWATT;

- dtpm->power_max = pd->table[pd->nr_perf_states - 1].power;
+ dtpm->power_max = table[pd->nr_perf_states - 1].power;
dtpm->power_max *= MICROWATT_PER_MILLIWATT;

return 0;
@@ -53,22 +56,25 @@ static u64 set_pd_power_limit(struct dtpm *dtpm, u64 power_limit)
struct devfreq *devfreq = dtpm_devfreq->devfreq;
struct device *dev = devfreq->dev.parent;
struct em_perf_domain *pd = em_pd_get(dev);
+ struct em_perf_state *table;
unsigned long freq;
u64 power;
int i;

+ table = pd->default_table->state;
+
for (i = 0; i < pd->nr_perf_states; i++) {

- power = pd->table[i].power * MICROWATT_PER_MILLIWATT;
+ power = table[i].power * MICROWATT_PER_MILLIWATT;
if (power > power_limit)
break;
}

- freq = pd->table[i - 1].frequency;
+ freq = table[i - 1].frequency;

dev_pm_qos_update_request(&dtpm_devfreq->qos_req, freq);

- power_limit = pd->table[i - 1].power * MICROWATT_PER_MILLIWATT;
+ power_limit = table[i - 1].power * MICROWATT_PER_MILLIWATT;

return power_limit;
}
@@ -94,6 +100,7 @@ static u64 get_pd_power_uw(struct dtpm *dtpm)
struct device *dev = devfreq->dev.parent;
struct em_perf_domain *pd = em_pd_get(dev);
struct devfreq_dev_status status;
+ struct em_perf_state *table;
unsigned long freq;
u64 power;
int i;
@@ -102,15 +109,17 @@ static u64 get_pd_power_uw(struct dtpm *dtpm)
status = devfreq->last_status;
mutex_unlock(&devfreq->lock);

+ table = pd->default_table->state;
+
freq = DIV_ROUND_UP(status.current_frequency, HZ_PER_KHZ);
_normalize_load(&status);

for (i = 0; i < pd->nr_perf_states; i++) {

- if (pd->table[i].frequency < freq)
+ if (table[i].frequency < freq)
continue;

- power = pd->table[i].power * MICROWATT_PER_MILLIWATT;
+ power = table[i].power * MICROWATT_PER_MILLIWATT;
power *= status.busy_time;
power >>= 10;

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index e2cc7bd30862..1d979c5e05ed 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -91,10 +91,11 @@ struct cpufreq_cooling_device {
static unsigned long get_level(struct cpufreq_cooling_device *cpufreq_cdev,
unsigned int freq)
{
+ struct em_perf_state *table = cpufreq_cdev->em->default_table->state;
int i;

for (i = cpufreq_cdev->max_level - 1; i >= 0; i--) {
- if (freq > cpufreq_cdev->em->table[i].frequency)
+ if (freq > table[i].frequency)
break;
}

@@ -104,15 +105,16 @@ static unsigned long get_level(struct cpufreq_cooling_device *cpufreq_cdev,
static u32 cpu_freq_to_power(struct cpufreq_cooling_device *cpufreq_cdev,
u32 freq)
{
+ struct em_perf_state *table = cpufreq_cdev->em->default_table->state;
unsigned long power_mw;
int i;

for (i = cpufreq_cdev->max_level - 1; i >= 0; i--) {
- if (freq > cpufreq_cdev->em->table[i].frequency)
+ if (freq > table[i].frequency)
break;
}

- power_mw = cpufreq_cdev->em->table[i + 1].power;
+ power_mw = table[i + 1].power;
power_mw /= MICROWATT_PER_MILLIWATT;

return power_mw;
@@ -121,18 +123,19 @@ static u32 cpu_freq_to_power(struct cpufreq_cooling_device *cpufreq_cdev,
static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev,
u32 power)
{
+ struct em_perf_state *table = cpufreq_cdev->em->default_table->state;
unsigned long em_power_mw;
int i;

for (i = cpufreq_cdev->max_level; i > 0; i--) {
/* Convert EM power to milli-Watts to make safe comparison */
- em_power_mw = cpufreq_cdev->em->table[i].power;
+ em_power_mw = table[i].power;
em_power_mw /= MICROWATT_PER_MILLIWATT;
if (power >= em_power_mw)
break;
}

- return cpufreq_cdev->em->table[i].frequency;
+ return table[i].frequency;
}

/**
@@ -262,8 +265,9 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
static int cpufreq_state2power(struct thermal_cooling_device *cdev,
unsigned long state, u32 *power)
{
- unsigned int freq, num_cpus, idx;
struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
+ unsigned int freq, num_cpus, idx;
+ struct em_perf_state *table;

/* Request state should be less than max_level */
if (state > cpufreq_cdev->max_level)
@@ -271,8 +275,9 @@ static int cpufreq_state2power(struct thermal_cooling_device *cdev,

num_cpus = cpumask_weight(cpufreq_cdev->policy->cpus);

+ table = cpufreq_cdev->em->default_table->state;
idx = cpufreq_cdev->max_level - state;
- freq = cpufreq_cdev->em->table[idx].frequency;
+ freq = table[idx].frequency;
*power = cpu_freq_to_power(cpufreq_cdev, freq) * num_cpus;

return 0;
@@ -378,8 +383,10 @@ static unsigned int get_state_freq(struct cpufreq_cooling_device *cpufreq_cdev,
#ifdef CONFIG_THERMAL_GOV_POWER_ALLOCATOR
/* Use the Energy Model table if available */
if (cpufreq_cdev->em) {
+ struct em_perf_state *table;
+ table = cpufreq_cdev->em->default_table->state;
idx = cpufreq_cdev->max_level - state;
- return cpufreq_cdev->em->table[idx].frequency;
+ return table[idx].frequency;
}
#endif

diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 262e62ab6cf2..4207ef850582 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -87,6 +87,7 @@ static int devfreq_cooling_set_cur_state(struct thermal_cooling_device *cdev,
struct devfreq_cooling_device *dfc = cdev->devdata;
struct devfreq *df = dfc->devfreq;
struct device *dev = df->dev.parent;
+ struct em_perf_state *table;
unsigned long freq;
int perf_idx;

@@ -99,8 +100,9 @@ static int devfreq_cooling_set_cur_state(struct thermal_cooling_device *cdev,
return -EINVAL;

if (dfc->em_pd) {
+ table = dfc->em_pd->default_table->state;
perf_idx = dfc->max_state - state;
- freq = dfc->em_pd->table[perf_idx].frequency * 1000;
+ freq = table[perf_idx].frequency * 1000;
} else {
freq = dfc->freq_table[state];
}
@@ -123,10 +125,11 @@ static int devfreq_cooling_set_cur_state(struct thermal_cooling_device *cdev,
*/
static int get_perf_idx(struct em_perf_domain *em_pd, unsigned long freq)
{
+ struct em_perf_state *table = em_pd->default_table->state;
int i;

for (i = 0; i < em_pd->nr_perf_states; i++) {
- if (em_pd->table[i].frequency == freq)
+ if (table[i].frequency == freq)
return i;
}

@@ -181,6 +184,7 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
struct devfreq_cooling_device *dfc = cdev->devdata;
struct devfreq *df = dfc->devfreq;
struct devfreq_dev_status status;
+ struct em_perf_state *table;
unsigned long state;
unsigned long freq;
unsigned long voltage;
@@ -192,6 +196,8 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd

freq = status.current_frequency;

+ table = dfc->em_pd->default_table->state;
+
if (dfc->power_ops && dfc->power_ops->get_real_power) {
voltage = get_voltage(df, freq);
if (voltage == 0) {
@@ -204,7 +210,7 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
state = dfc->capped_state;

/* Convert EM power into milli-Watts first */
- dfc->res_util = dfc->em_pd->table[state].power;
+ dfc->res_util = table[state].power;
dfc->res_util /= MICROWATT_PER_MILLIWATT;

dfc->res_util *= SCALE_ERROR_MITIGATION;
@@ -225,7 +231,7 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
_normalize_load(&status);

/* Convert EM power into milli-Watts first */
- *power = dfc->em_pd->table[perf_idx].power;
+ *power = table[perf_idx].power;
*power /= MICROWATT_PER_MILLIWATT;
/* Scale power for utilization */
*power *= status.busy_time;
@@ -245,13 +251,15 @@ static int devfreq_cooling_state2power(struct thermal_cooling_device *cdev,
unsigned long state, u32 *power)
{
struct devfreq_cooling_device *dfc = cdev->devdata;
+ struct em_perf_state *table;
int perf_idx;

if (state > dfc->max_state)
return -EINVAL;

+ table = dfc->em_pd->default_table->state;
perf_idx = dfc->max_state - state;
- *power = dfc->em_pd->table[perf_idx].power;
+ *power = table[perf_idx].power;
*power /= MICROWATT_PER_MILLIWATT;

return 0;
@@ -264,6 +272,7 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
struct devfreq *df = dfc->devfreq;
struct devfreq_dev_status status;
unsigned long freq, em_power_mw;
+ struct em_perf_state *table;
s32 est_power;
int i;

@@ -273,6 +282,8 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,

freq = status.current_frequency;

+ table = dfc->em_pd->default_table->state;
+
if (dfc->power_ops && dfc->power_ops->get_real_power) {
/* Scale for resource utilization */
est_power = power * dfc->res_util;
@@ -290,7 +301,7 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
*/
for (i = dfc->max_state; i > 0; i--) {
/* Convert EM power to milli-Watts to make safe comparison */
- em_power_mw = dfc->em_pd->table[i].power;
+ em_power_mw = table[i].power;
em_power_mw /= MICROWATT_PER_MILLIWATT;
if (est_power >= em_power_mw)
break;
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index b8506df9af2d..eb28920b1b2c 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -49,7 +49,7 @@ struct em_perf_table {

/**
* struct em_perf_domain - Performance domain
- * @table: List of performance states, in ascending order
+ * @default_table: Pointer to the default em_perf_table
* @runtime_table: Pointer to the runtime modified em_perf_table
* @nr_perf_states: Number of performance states
* @flags: See "em_perf_domain flags"
@@ -65,7 +65,7 @@ struct em_perf_table {
* field is unused.
*/
struct em_perf_domain {
- struct em_perf_state *table;
+ struct em_perf_table *default_table;
struct em_perf_table __rcu *runtime_table;
int nr_perf_states;
unsigned long flags;
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 456d9f2b4370..204fd415ebc9 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -72,6 +72,7 @@ DEFINE_SHOW_ATTRIBUTE(em_debug_flags);

static void em_debug_create_pd(struct device *dev)
{
+ struct em_perf_table *table = dev->em_pd->default_table;
struct dentry *d;
int i;

@@ -87,7 +88,7 @@ static void em_debug_create_pd(struct device *dev)

/* Create a sub-directory for each performance state */
for (i = 0; i < dev->em_pd->nr_perf_states; i++)
- em_debug_create_ps(&dev->em_pd->table[i], d);
+ em_debug_create_ps(&table->state[i], d);

}

@@ -157,7 +158,7 @@ static void em_perf_runtime_table_set(struct device *dev,
* 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)
+ if (tmp->state == pd->default_table->state)
call_rcu(&tmp->rcu, em_destroy_tmp_setup_rcu);
else
call_rcu(&tmp->rcu, em_destroy_rt_table_rcu);
@@ -261,7 +262,7 @@ int em_dev_update_perf_domain(struct device *dev, struct em_data_callback *cb,

/* Populate runtime table with updated values using driver callback */
for (i = 0; i < pd->nr_perf_states; i++) {
- freq = pd->table[i].frequency;
+ freq = pd->default_table->state[i].frequency;
runtime_table->state[i].frequency = freq;

/*
@@ -352,7 +353,7 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
if (ret)
goto free_ps_table;

- pd->table = table;
+ pd->default_table->state = table;
pd->nr_perf_states = nr_states;

return 0;
@@ -366,7 +367,7 @@ static int em_create_pd(struct device *dev, int nr_states,
struct em_data_callback *cb, cpumask_t *cpus,
unsigned long flags)
{
- struct em_perf_table *runtime_table;
+ struct em_perf_table *default_table, *runtime_table;
struct em_perf_domain *pd;
struct device *cpu_dev;
int cpu, ret, num_cpus;
@@ -391,21 +392,31 @@ static int em_create_pd(struct device *dev, int nr_states,
return -ENOMEM;
}

+ default_table = kzalloc(sizeof(*default_table), GFP_KERNEL);
+ if (!default_table) {
+ kfree(pd);
+ return -ENOMEM;
+ }
+
runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
if (!runtime_table) {
kfree(pd);
+ kfree(default_table);
return -ENOMEM;
}

+ pd->default_table = default_table;
+
ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
if (ret) {
kfree(pd);
+ kfree(default_table);
kfree(runtime_table);
return ret;
}

/* Re-use temporally (till 1st modification) the memory */
- runtime_table->state = pd->table;
+ runtime_table->state = pd->default_table->state;
rcu_assign_pointer(pd->runtime_table, runtime_table);

if (_is_cpu_device(dev))
@@ -526,6 +537,7 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
bool microwatts)
{
unsigned long cap, prev_cap = 0;
+ struct em_perf_state *table;
unsigned long flags = 0;
int cpu, ret;

@@ -584,7 +596,8 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,

dev->em_pd->flags |= flags;

- em_cpufreq_update_efficiencies(dev, dev->em_pd->table);
+ table = dev->em_pd->default_table->state;
+ em_cpufreq_update_efficiencies(dev, table);

em_debug_create_pd(dev);
dev_info(dev, "EM: created perf domain\n");
@@ -638,12 +651,13 @@ void em_dev_unregister_perf_domain(struct device *dev)
* After the sync no updates will be in-flight, so free the old
* memory.
*/
- if (tmp->state != pd->table)
+ if (tmp->state != pd->default_table->state)
kfree(tmp->state);

kfree(tmp);

- kfree(dev->em_pd->table);
+ kfree(pd->default_table->state);
+ kfree(pd->default_table);
kfree(dev->em_pd);
dev->em_pd = NULL;
mutex_unlock(&em_pd_mutex);
--
2.25.1


2023-05-12 10:10:56

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH v2 06/17] PM: EM: Add update_power() callback for runtime modifications

The Energy Model (EM) is going to support runtime modifications. This
new callback would be used in the upcoming EM changes. The drivers
or frameworks which want to modify the EM have to implement the
update_power() callback and provide it via EM API
em_dev_update_perf_domain(). The callback is then used by the EM
framework to get new power values for each frequency in existing EM.

Signed-off-by: Lukasz Luba <[email protected]>
---
include/linux/energy_model.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index 8069f526c9d8..cc2bf607191e 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -158,6 +158,26 @@ struct em_data_callback {
*/
int (*get_cost)(struct device *dev, unsigned long freq,
unsigned long *cost);
+
+ /**
+ * update_power() - Provide new power at the given performance state of
+ * a device
+ * @dev : Device for which we do this operation (can be a CPU)
+ * @freq : Frequency at the performance state in kHz
+ * @power : New power value at the performance state
+ * (modified)
+ * @priv : Pointer to private data useful for tracking context
+ * during run-time modifications of EM.
+ *
+ * The update_power() is used by run-time modifiable EM. It aims to
+ * provide updated power value for a given frequency, which is stored
+ * in the performance state. The power value provided by this callback
+ * should fit in the [0, EM_MAX_POWER] range.
+ *
+ * Return 0 on success, or appropriate error value in case of failure.
+ */
+ int (*update_power)(struct device *dev, unsigned long freq,
+ unsigned long *power, void *priv);
};
#define EM_SET_ACTIVE_POWER_CB(em_cb, cb) ((em_cb).active_power = cb)
#define EM_ADV_DATA_CB(_active_power_cb, _cost_cb) \
@@ -165,6 +185,7 @@ struct em_data_callback {
.get_cost = _cost_cb }
#define EM_DATA_CB(_active_power_cb) \
EM_ADV_DATA_CB(_active_power_cb, NULL)
+#define EM_UPDATE_CB(_update_power_cb) { .update_power = &_update_power_cb }

struct em_perf_domain *em_cpu_get(int cpu);
struct em_perf_domain *em_pd_get(struct device *dev);
--
2.25.1


2023-05-14 05:50:48

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 08/17] PM: EM: Introduce runtime modifiable table

Hi Lukasz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/thermal linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Lukasz-Luba/PM-EM-Refactor-em_cpufreq_update_efficiencies-arguments/20230512-180158
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230512095743.3393563-9-lukasz.luba%40arm.com
patch subject: [PATCH v2 08/17] PM: EM: Introduce runtime modifiable table
config: arm64-randconfig-s041-20230514 (https://download.01.org/0day-ci/archive/20230514/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/d12d8d1010d7b093d6b64c204d77484d6fc268ab
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Lukasz-Luba/PM-EM-Refactor-em_cpufreq_update_efficiencies-arguments/20230512-180158
git checkout d12d8d1010d7b093d6b64c204d77484d6fc268ab
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash kernel/power/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)
>> kernel/power/energy_model.c:472:13: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct em_perf_table *tmp @@ got struct em_perf_table [noderef] __rcu *runtime_table @@
kernel/power/energy_model.c:472:13: sparse: expected struct em_perf_table *tmp
kernel/power/energy_model.c:472:13: sparse: got struct em_perf_table [noderef] __rcu *runtime_table

vim +472 kernel/power/energy_model.c

444
445 /**
446 * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
447 * @dev : Device for which the EM is registered
448 *
449 * Unregister the EM for the specified @dev (but not a CPU device).
450 */
451 void em_dev_unregister_perf_domain(struct device *dev)
452 {
453 struct em_perf_domain *pd;
454 struct em_perf_table *tmp;
455
456 if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
457 return;
458
459 if (_is_cpu_device(dev))
460 return;
461
462 pd = dev->em_pd;
463 /*
464 * The mutex separates all register/unregister requests and protects
465 * from potential clean-up/setup issues in the debugfs directories.
466 * The debugfs directory name is the same as device's name.
467 */
468 mutex_lock(&em_pd_mutex);
469
470 em_debug_remove_pd(dev);
471
> 472 tmp = pd->runtime_table;

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-05-24 17:40:13

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 00/17] Introduce runtime modifiable Energy Model

Hi Lukasz,

On Fri, May 12, 2023 at 11:58 AM Lukasz Luba <[email protected]> wrote:
>
> 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's part of feature set know as Dynamic Energy Model. It has been presented
> and discussed recently at OSPM2023 [3]. This patch set implements the 1st
> improvement for the EM.
>
> 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 or EM can be adjusted and reflect the fact of increased
> power usage. That power increase is due to a factor called 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'.
>
> Some design details:
> The internal mechanisms for the memory allocation are handled internally in the
> EM. Kernel modules can just call the new API to update the EM data and the
> new memory would be provided and owned by the EM. The EM memory is used by
> EAS, which impacts those design decisions. The EM writers are protected by
> a mutex. This new runtime modified EM table is protected using RCU mechanism,
> which fits the current EAS hot path (which already uses RCU read lock).
> The unregister API handles only non-CPU (e.g. GPU, ISP) devices and uses the
> same mutex as EM modifiers to make sure the memory is safely freed.
>
> More detailed explanation and background can be found in presentations
> during LPC2022 [1][2] or in the documentation patches.
>
> Changelog:
> v2:
> - solved build warning of unused variable in patch 13/17 when EM is
> not compiled in, e.g. on Intel platform for this cpufreq_cooling
> - re-based on top of v6.4-rc1
> v1:
> - implementation can be found here [4]
>
> [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/[email protected]/
>
> Lukasz Luba (17):
> PM: EM: Refactor em_cpufreq_update_efficiencies() arguments
> PM: EM: Find first CPU online while updating OPP efficiency
> PM: EM: Refactor em_pd_get_efficient_state() to be more flexible
> PM: EM: Create a new function em_compute_costs()
> trace: energy_model: Add trace event for EM runtime modifications
> PM: EM: Add update_power() callback for runtime modifications
> PM: EM: Check if the get_cost() callback is present in
> em_compute_costs()
> PM: EM: Introduce runtime modifiable table
> PM: EM: Add RCU mechanism which safely cleans the old data
> PM: EM: Add runtime update interface to modify EM power
> PM: EM: Use runtime modified EM for CPUs energy estimation in EAS
> PM: EM: Add argument to get_cost() for runtime modification
> PM: EM: Refactor struct em_perf_domain and add default_table
> Documentation: EM: Add a new section about the design
> Documentation: EM: Add a runtime modifiable EM design description
> Documentation: EM: Add example with driver modifying the EM
> Documentation: EM: Describe the API of runtime modifications

I haven't seen any responses from anyone having a vested interest in
the Energy Model code.

I'm not sure what this means, but I surely can't do much about it
myself without any input from the potentially interested parties.

Thanks!

2023-05-30 09:57:45

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 06/17] PM: EM: Add update_power() callback for runtime modifications

On 12/05/2023 11:57, Lukasz Luba wrote:
> The Energy Model (EM) is going to support runtime modifications. This
> new callback would be used in the upcoming EM changes. The drivers
> or frameworks which want to modify the EM have to implement the
> update_power() callback and provide it via EM API
> em_dev_update_perf_domain(). The callback is then used by the EM
> framework to get new power values for each frequency in existing EM.

Do we have any numbers or feedback that the chosen design (i.e. update
per performance state through update_power()) is performant enough for
the anticipated use case on real devices?

> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> include/linux/energy_model.h | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> index 8069f526c9d8..cc2bf607191e 100644
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -158,6 +158,26 @@ struct em_data_callback {
> */
> int (*get_cost)(struct device *dev, unsigned long freq,
> unsigned long *cost);
> +
> + /**
> + * update_power() - Provide new power at the given performance state of
> + * a device
> + * @dev : Device for which we do this operation (can be a CPU)
> + * @freq : Frequency at the performance state in kHz
> + * @power : New power value at the performance state
> + * (modified)
> + * @priv : Pointer to private data useful for tracking context
> + * during run-time modifications of EM.
> + *
> + * The update_power() is used by run-time modifiable EM. It aims to
> + * provide updated power value for a given frequency, which is stored
> + * in the performance state. The power value provided by this callback
> + * should fit in the [0, EM_MAX_POWER] range.
> + *
> + * Return 0 on success, or appropriate error value in case of failure.
> + */
> + int (*update_power)(struct device *dev, unsigned long freq,
> + unsigned long *power, void *priv);
> };
> #define EM_SET_ACTIVE_POWER_CB(em_cb, cb) ((em_cb).active_power = cb)
> #define EM_ADV_DATA_CB(_active_power_cb, _cost_cb) \
> @@ -165,6 +185,7 @@ struct em_data_callback {
> .get_cost = _cost_cb }
> #define EM_DATA_CB(_active_power_cb) \
> EM_ADV_DATA_CB(_active_power_cb, NULL)
> +#define EM_UPDATE_CB(_update_power_cb) { .update_power = &_update_power_cb }
>
> struct em_perf_domain *em_cpu_get(int cpu);
> struct em_perf_domain *em_pd_get(struct device *dev);


2023-05-30 10:21:20

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 12/17] PM: EM: Add argument to get_cost() for runtime modification

On 12/05/2023 11:57, Lukasz Luba wrote:
> The Energy Model (EM) supports runtime modifications. Let also the
> artificial EM use this new feature and allow to update the 'cost' values
> at runtime. When the artificial EM is used there is a need to provide
> two callbacks: get_cost() and update_power(), not only the last one.
>
> Update also CPPC driver code, since the new argument is needed there
> to compile properly and register EM.

Is there a real use case behind this? It can't be mobile which IMHO
drivers the rest of the 'Runtime modifiable EM' feature.

Do we know of any machine using the artificial EM. And do they care
about EM matching workload or static power?

[...]

2023-05-30 10:33:02

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 13/17] PM: EM: Refactor struct em_perf_domain and add default_table

On 12/05/2023 11:57, Lukasz Luba wrote:
> The Energy Model support runtime modifications. Refactor old
> implementation which accessed struct em_perf_state and introduce
> em_perf_domain::default_table to clean up the design. This new field
> is better aligned with em_perf_domain::runtime_table and helps to
> distinguish them better.
>
> Update all drivers or frameworks which used the old field:
> em_perf_domain::table and now should use em_perf_domain::default_table.

I still believe that doing this refactoring before the introducation o
the new feature `runtime modifiable EM` would do wonders on the
tangibility of the other patches in this set.

[...]


2023-05-30 10:49:54

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 08/17] PM: EM: Introduce runtime modifiable table

On 12/05/2023 11:57, Lukasz Luba wrote:
> This patch introduces the new feature: modifiable EM perf_state table.
> The new runtime table would be populated with a new power data to better
> reflect the actual power. The power can vary over time e.g. due to the
> SoC temperature change. Higher temperature can increase power values.
> For longer running scenarios, such as game or camera, when also other
> devices are used (e.g. GPU, ISP) the CPU power can change. The new
> EM framework is able to addresses this issue and change the data
> at runtime safely. The runtime modifiable EM data is used by the Energy
> Aware Scheduler (EAS) for the task placement.

It's important to say that EAS is the _only_user of the `runtime
modifiable EM`. All the other users (thermal, etc.) are still using the
default (basic) EM. IMHO, this fact drove the design here.

> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> include/linux/energy_model.h | 13 +++++++++++++
> kernel/power/energy_model.c | 24 ++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> index cc2bf607191e..a616006a8130 100644
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -36,9 +36,21 @@ struct em_perf_state {
> */
> #define EM_PERF_STATE_INEFFICIENT BIT(0)
>
> +/**
> + * struct em_perf_table - Performance states table, which can be
> + * runtime modifiable and protected with RCU

which is `runtime modifiable` ? So `runtime modifiable performance state
table`? RCU is obvious since we have `struct rcu_head rcu`.

> + * @state: List of performance states, in ascending order
> + * @rcu: RCU used for safe access and destruction
> + */
> +struct em_perf_table {
> + struct em_perf_state *state;
> + struct rcu_head rcu;
> +};
> +
> /**
> * struct em_perf_domain - Performance domain
> * @table: List of performance states, in ascending order
> + * @runtime_table: Pointer to the runtime modified em_perf_table

s/modified/modifiable

[...]

> @@ -237,12 +238,23 @@ static int em_create_pd(struct device *dev, int nr_states,
> return -ENOMEM;
> }
>
> + runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
> + if (!runtime_table) {
> + kfree(pd);
> + return -ENOMEM;
> + }
> +
> ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
> if (ret) {
> kfree(pd);
> + kfree(runtime_table);
> return ret;
> }
>
> + /* Re-use temporally (till 1st modification) the memory */

So this means that the runtime (modifiable) table
(pd->runtime_table>state) is mapped to the default (basic) table
(pd->default_table->state) until the first call to
em_dev_update_perf_domain() (here mentioned as the 1st modification)?

IMHO, not easy to understand since neither the cover letter, nor
documentation patch 15/17 describes this in a consistent story.

[...]

2023-05-30 10:54:38

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 15/17] Documentation: EM: Add a runtime modifiable EM design description

On 12/05/2023 11:57, Lukasz Luba wrote:
> Document the new runtime modifiable EM design and how it can be used.
> Change the last section layout and allow to provide another example
> how to use this new API in a driver code.
>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> Documentation/power/energy-model.rst | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/power/energy-model.rst b/Documentation/power/energy-model.rst
> index e97c7f18d8bd..64c2462dc9a6 100644
> --- a/Documentation/power/energy-model.rst
> +++ b/Documentation/power/energy-model.rst
> @@ -89,6 +89,26 @@ due to the workload instruction mix and data set is not modeled.
> Also static power which can change during runtime due to variation of SOC
> temperature is not modeled in EM.
>
> +2.2 Runtime modifiable EM
> +^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +To better reflect power variation due to static power (leakage) the EM
> +supports runtime modifications of the power values. The mechanism relies on
> +RCU to free the modifiable EM perf_state table memory. Its user, the task
> +scheduler, also uses RCU to access this memory. The EM framework is
> +responsible for allocating the new memory for the modifiable EM perf_state
> +table. The old memory is freed automatically using RCU callback mechanism.
> +This design decision is made based on task scheduler using that data and
> +to prevent wrong usage of kernel modules if they would be responsible for the
> +memory management.
> +The drivers which want to modify the EM values are protected from concurrent
> +access using a mutex. Therefore, the drivers must use sleeping context when
> +they want to modify the EM. The runtime modifiable EM might also be used for
> +better reflecting real workload scenarios, e.g. when they pop-up on the screen
> +and will run for longer period, such as: games, video recoding or playing,
> +video calls, etc. It is up to the platform engineers to experiment and choose
> +the right approach for their device.

IMHO, there are a lot of design aspects missing here.

E.g.

Why 2 tables, modifiable (a) and default (b)?

Why does only EAS use (a)?

(a) and (b) being the same performance state table until first call to
modify (a) ()

> +
>
> 3. Core APIs
> ------------


2023-05-30 11:39:12

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH v2 00/17] Introduce runtime modifiable Energy Model

On 12/05/2023 11:57, Lukasz Luba wrote:
> 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's part of feature set know as Dynamic Energy Model. It has been presented
> and discussed recently at OSPM2023 [3]. This patch set implements the 1st
> improvement for the EM.

Why is the feature set called Dynamic Energy Model?

Dynamic Energy Model:

Runtime modifiable EM

Proper CPU performance state evaluation

CPU idle wakeup costs

CPU capacity as new EM data

Didn't this `Dynamic` stand for the modifiability of the EM only?

> 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 or EM can be adjusted and reflect the fact of increased
> power usage. That power increase is due to a factor called 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'.
>
> Some design details:
> The internal mechanisms for the memory allocation are handled internally in the
> EM. Kernel modules can just call the new API to update the EM data and the
> new memory would be provided and owned by the EM. The EM memory is used by
> EAS, which impacts those design decisions. The EM writers are protected by
> a mutex. This new runtime modified EM table is protected using RCU mechanism,
> which fits the current EAS hot path (which already uses RCU read lock).
> The unregister API handles only non-CPU (e.g. GPU, ISP) devices and uses the
> same mutex as EM modifiers to make sure the memory is safely freed.

This only mentions memory allocation and locking? A global design
overview, containing e.g.

Why 2 tables, modifiable (a) and default (b)?

Why does only EAS use (a)?

(a) and (b) being the same performance state table until first call to
modify (a) ()

as an introduction into the patches would be more helpful here.

> More detailed explanation and background can be found in presentations
> during LPC2022 [1][2] or in the documentation patches.

I checked 15/17 as well but could find any of this information there either.

[...]

2023-07-03 11:21:53

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 00/17] Introduce runtime modifiable Energy Model

Hi Rafael,


On 5/24/23 18:25, Rafael J. Wysocki wrote:
> Hi Lukasz,
>
> On Fri, May 12, 2023 at 11:58 AM Lukasz Luba <[email protected]> wrote:
>>
>> 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's part of feature set know as Dynamic Energy Model. It has been presented
>> and discussed recently at OSPM2023 [3]. This patch set implements the 1st
>> improvement for the EM.
>>
>> 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 or EM can be adjusted and reflect the fact of increased
>> power usage. That power increase is due to a factor called 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'.
>>
>> Some design details:
>> The internal mechanisms for the memory allocation are handled internally in the
>> EM. Kernel modules can just call the new API to update the EM data and the
>> new memory would be provided and owned by the EM. The EM memory is used by
>> EAS, which impacts those design decisions. The EM writers are protected by
>> a mutex. This new runtime modified EM table is protected using RCU mechanism,
>> which fits the current EAS hot path (which already uses RCU read lock).
>> The unregister API handles only non-CPU (e.g. GPU, ISP) devices and uses the
>> same mutex as EM modifiers to make sure the memory is safely freed.
>>
>> More detailed explanation and background can be found in presentations
>> during LPC2022 [1][2] or in the documentation patches.
>>
>> Changelog:
>> v2:
>> - solved build warning of unused variable in patch 13/17 when EM is
>> not compiled in, e.g. on Intel platform for this cpufreq_cooling
>> - re-based on top of v6.4-rc1
>> v1:
>> - implementation can be found here [4]
>>
>> [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/[email protected]/
>>
>> Lukasz Luba (17):
>> PM: EM: Refactor em_cpufreq_update_efficiencies() arguments
>> PM: EM: Find first CPU online while updating OPP efficiency
>> PM: EM: Refactor em_pd_get_efficient_state() to be more flexible
>> PM: EM: Create a new function em_compute_costs()
>> trace: energy_model: Add trace event for EM runtime modifications
>> PM: EM: Add update_power() callback for runtime modifications
>> PM: EM: Check if the get_cost() callback is present in
>> em_compute_costs()
>> PM: EM: Introduce runtime modifiable table
>> PM: EM: Add RCU mechanism which safely cleans the old data
>> PM: EM: Add runtime update interface to modify EM power
>> PM: EM: Use runtime modified EM for CPUs energy estimation in EAS
>> PM: EM: Add argument to get_cost() for runtime modification
>> PM: EM: Refactor struct em_perf_domain and add default_table
>> Documentation: EM: Add a new section about the design
>> Documentation: EM: Add a runtime modifiable EM design description
>> Documentation: EM: Add example with driver modifying the EM
>> Documentation: EM: Describe the API of runtime modifications
>
> I haven't seen any responses from anyone having a vested interest in
> the Energy Model code.
>
> I'm not sure what this means, but I surely can't do much about it
> myself without any input from the potentially interested parties.

My apologies for the delay. Correct, it has been missing attention,
but now Dietmar is reviewing the stuff. He commented a few things
and I'm going to respond and address them in v3.

Thanks for having a look into this thread!

Lukasz

2023-07-03 15:08:04

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 06/17] PM: EM: Add update_power() callback for runtime modifications

Hi Dietmar,

On 5/30/23 10:31, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> The Energy Model (EM) is going to support runtime modifications. This
>> new callback would be used in the upcoming EM changes. The drivers
>> or frameworks which want to modify the EM have to implement the
>> update_power() callback and provide it via EM API
>> em_dev_update_perf_domain(). The callback is then used by the EM
>> framework to get new power values for each frequency in existing EM.
>
> Do we have any numbers or feedback that the chosen design (i.e. update
> per performance state through update_power()) is performant enough for
> the anticipated use case on real devices?
>

Yes, we have. I have a testing kernel module which updates the EM
with queue_delayed_work() every 100ms. That update is for Little's EM
where we have 11 OPPs. We call the new callback for each OPP
in the em_dev_update_perf_domain(). I have measured that total function
time.

When we fix all CPUs freq to max freq on pixel6 and disable deep idle
states and leave only WFI, then we can run some tracing and capture the
results:

(The 4 CPUs from top are the little (1.8MHz), than 2 Mid (2.2GHz) and
then 2 big (2.8GHz))
------------------------------------
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 3104 51236.39 us 16.506
us 75.344 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 1264 20768.15 us 16.430
us 62.257 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 1166 18632.95 us 15.980
us 70.707 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 770 12334.43 us 16.018
us 66.337 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 101 920.613 us 9.114
us 21.380 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 20 211.830 us 10.591
us 23.998 us
Function Hit Time Avg
s^2
-------- --- ---- ---
---
Function Hit Time Avg
s^2
-------- --- ---- ---
---
em_dev_update_perf_domain 15 78.085 us 5.205
us 7.444 us

------------------------------------

As you can see in avg on Little CPUs it takes ~16us, on Mid ~10us and on
Big ~5us.

If such updating kernel module is implemented correctly, it would be
most often scheduled on the Littles as you can see based on 'Hit'
column.

Therefore, IMO this cost can be OK for the upstream. This EM runtime
change won't be triggered very often. If it would be e.g. every
100ms than the cost ~1.5us per 1 OPP is negligible.

Regards,
Lukasz

2023-07-03 15:59:25

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 12/17] PM: EM: Add argument to get_cost() for runtime modification



On 5/30/23 10:53, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> The Energy Model (EM) supports runtime modifications. Let also the
>> artificial EM use this new feature and allow to update the 'cost' values
>> at runtime. When the artificial EM is used there is a need to provide
>> two callbacks: get_cost() and update_power(), not only the last one.
>>
>> Update also CPPC driver code, since the new argument is needed there
>> to compile properly and register EM.
>
> Is there a real use case behind this? It can't be mobile which IMHO
> drivers the rest of the 'Runtime modifiable EM' feature.

Correct, CPPC+EM is not for mobile phones, but notebooks. For now
the notebooks are not tested with that feature and were not in
requirement scope.

>
> Do we know of any machine using the artificial EM. And do they care
> about EM matching workload or static power?
>
> [...]

For now we don't know about such notebook which uses CPPC + EM
and if it's a candidate for this feature.

I thought, since it's just one patch w/ small change, it would be
consistent to not 'forget' about that notebooks angle. I know
that there are some folks who 'run'/'are willing to run' Arm laptop with
CPPC w/ artificial EM. They might pick that feature as well.

2023-07-03 16:19:07

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 15/17] Documentation: EM: Add a runtime modifiable EM design description



On 5/30/23 11:42, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> Document the new runtime modifiable EM design and how it can be used.
>> Change the last section layout and allow to provide another example
>> how to use this new API in a driver code.
>>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> Documentation/power/energy-model.rst | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/power/energy-model.rst b/Documentation/power/energy-model.rst
>> index e97c7f18d8bd..64c2462dc9a6 100644
>> --- a/Documentation/power/energy-model.rst
>> +++ b/Documentation/power/energy-model.rst
>> @@ -89,6 +89,26 @@ due to the workload instruction mix and data set is not modeled.
>> Also static power which can change during runtime due to variation of SOC
>> temperature is not modeled in EM.
>>
>> +2.2 Runtime modifiable EM
>> +^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +To better reflect power variation due to static power (leakage) the EM
>> +supports runtime modifications of the power values. The mechanism relies on
>> +RCU to free the modifiable EM perf_state table memory. Its user, the task
>> +scheduler, also uses RCU to access this memory. The EM framework is
>> +responsible for allocating the new memory for the modifiable EM perf_state
>> +table. The old memory is freed automatically using RCU callback mechanism.
>> +This design decision is made based on task scheduler using that data and
>> +to prevent wrong usage of kernel modules if they would be responsible for the
>> +memory management.
>> +The drivers which want to modify the EM values are protected from concurrent
>> +access using a mutex. Therefore, the drivers must use sleeping context when
>> +they want to modify the EM. The runtime modifiable EM might also be used for
>> +better reflecting real workload scenarios, e.g. when they pop-up on the screen
>> +and will run for longer period, such as: games, video recoding or playing,
>> +video calls, etc. It is up to the platform engineers to experiment and choose
>> +the right approach for their device.
>
> IMHO, there are a lot of design aspects missing here.
>
> E.g.
>
> Why 2 tables, modifiable (a) and default (b)?
>
> Why does only EAS use (a)?
>
> (a) and (b) being the same performance state table until first call to
> modify (a) ()
>

I'll add that explanation in the v3. I wanted to avoid such detailed
description about e.g. 'being the same performance state table until
first call to > modify (a)' since it's a memory optimization
bit. Although, I will add that reason to the doc as well.

2023-07-03 16:25:53

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 08/17] PM: EM: Introduce runtime modifiable table



On 5/30/23 11:18, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> This patch introduces the new feature: modifiable EM perf_state table.
>> The new runtime table would be populated with a new power data to better
>> reflect the actual power. The power can vary over time e.g. due to the
>> SoC temperature change. Higher temperature can increase power values.
>> For longer running scenarios, such as game or camera, when also other
>> devices are used (e.g. GPU, ISP) the CPU power can change. The new
>> EM framework is able to addresses this issue and change the data
>> at runtime safely. The runtime modifiable EM data is used by the Energy
>> Aware Scheduler (EAS) for the task placement.
>
> It's important to say that EAS is the _only_user of the `runtime
> modifiable EM`. All the other users (thermal, etc.) are still using the
> default (basic) EM. IMHO, this fact drove the design here.

OK, I'll add that information in the header.

>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> include/linux/energy_model.h | 13 +++++++++++++
>> kernel/power/energy_model.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 37 insertions(+)
>>
>> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
>> index cc2bf607191e..a616006a8130 100644
>> --- a/include/linux/energy_model.h
>> +++ b/include/linux/energy_model.h
>> @@ -36,9 +36,21 @@ struct em_perf_state {
>> */
>> #define EM_PERF_STATE_INEFFICIENT BIT(0)
>>
>> +/**
>> + * struct em_perf_table - Performance states table, which can be
>> + * runtime modifiable and protected with RCU
>
> which is `runtime modifiable` ? So `runtime modifiable performance state
> table`? RCU is obvious since we have `struct rcu_head rcu`.

Thanks, 'Runtime modifiable performance state table' sounds better.

>
>> + * @state: List of performance states, in ascending order
>> + * @rcu: RCU used for safe access and destruction
>> + */
>> +struct em_perf_table {
>> + struct em_perf_state *state;
>> + struct rcu_head rcu;
>> +};
>> +
>> /**
>> * struct em_perf_domain - Performance domain
>> * @table: List of performance states, in ascending order
>> + * @runtime_table: Pointer to the runtime modified em_perf_table
>
> s/modified/modifiable
>
> [...]
>
>> @@ -237,12 +238,23 @@ static int em_create_pd(struct device *dev, int nr_states,
>> return -ENOMEM;
>> }
>>
>> + runtime_table = kzalloc(sizeof(*runtime_table), GFP_KERNEL);
>> + if (!runtime_table) {
>> + kfree(pd);
>> + return -ENOMEM;
>> + }
>> +
>> ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
>> if (ret) {
>> kfree(pd);
>> + kfree(runtime_table);
>> return ret;
>> }
>>
>> + /* Re-use temporally (till 1st modification) the memory */
>
> So this means that the runtime (modifiable) table
> (pd->runtime_table>state) is mapped to the default (basic) table
> (pd->default_table->state) until the first call to
> em_dev_update_perf_domain() (here mentioned as the 1st modification)?

correct

>
> IMHO, not easy to understand since neither the cover letter, nor
> documentation patch 15/17 describes this in a consistent story.

I'll add that to the patch header and also to the documentation patch
which is later in the series.


2023-07-03 16:41:53

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 13/17] PM: EM: Refactor struct em_perf_domain and add default_table



On 5/30/23 11:23, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> The Energy Model support runtime modifications. Refactor old
>> implementation which accessed struct em_perf_state and introduce
>> em_perf_domain::default_table to clean up the design. This new field
>> is better aligned with em_perf_domain::runtime_table and helps to
>> distinguish them better.
>>
>> Update all drivers or frameworks which used the old field:
>> em_perf_domain::table and now should use em_perf_domain::default_table.
>
> I still believe that doing this refactoring before the introducation o
> the new feature `runtime modifiable EM` would do wonders on the
> tangibility of the other patches in this set.
>
> [...]
>

Let me move that refactor patch a bit earlier in the series...

2023-07-03 17:06:28

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v2 00/17] Introduce runtime modifiable Energy Model



On 5/30/23 12:07, Dietmar Eggemann wrote:
> On 12/05/2023 11:57, Lukasz Luba wrote:
>> 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's part of feature set know as Dynamic Energy Model. It has been presented
>> and discussed recently at OSPM2023 [3]. This patch set implements the 1st
>> improvement for the EM.
>
> Why is the feature set called Dynamic Energy Model?
>
> Dynamic Energy Model:
>
> Runtime modifiable EM
>
> Proper CPU performance state evaluation
>
> CPU idle wakeup costs
>
> CPU capacity as new EM data
>
> Didn't this `Dynamic` stand for the modifiability of the EM only?

The 'modifiability' is the main feature, but not the last one.

The 2nd: 'Proper CPU performance state evaluation' is also
a 'dynamic' thing, which is related to the currently set CPU
frequency (for Mid or Big) and the dependent Little's and L3
cache frequency. As you know that frequency can change in time,
so it's 'dynamic' situation and will lands to the 'Dynamic EM'.

(The 2 below need more thinking and experiments)
The 3rd can be also quite dynamic. You might change the wake-up cost
for in the EM if you want to avoid waking up big cores in some
workloads. You might pay penalty for bigger latency, because tasks
would be more queued on Mids/Littles, but that's for power
saving scenario.

The 4th is about CPU capacity. We still have to conduct more
investigations, but it might be useful to change the EM
and provide a new CPU capacity from it to the OS. In the 3-gear
SoC the CPUs might have quite different capacity in different
scenarios (workloads). This could be also a 'dynamic' thing,
e.g. triggered by middle-ware for a long running video call.

>
>> 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 or EM can be adjusted and reflect the fact of increased
>> power usage. That power increase is due to a factor called 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'.
>>
>> Some design details:
>> The internal mechanisms for the memory allocation are handled internally in the
>> EM. Kernel modules can just call the new API to update the EM data and the
>> new memory would be provided and owned by the EM. The EM memory is used by
>> EAS, which impacts those design decisions. The EM writers are protected by
>> a mutex. This new runtime modified EM table is protected using RCU mechanism,
>> which fits the current EAS hot path (which already uses RCU read lock).
>> The unregister API handles only non-CPU (e.g. GPU, ISP) devices and uses the
>> same mutex as EM modifiers to make sure the memory is safely freed.
>
> This only mentions memory allocation and locking? A global design
> overview, containing e.g.
>
> Why 2 tables, modifiable (a) and default (b)?
>
> Why does only EAS use (a)?
>
> (a) and (b) being the same performance state table until first call to
> modify (a) ()
>
> as an introduction into the patches would be more helpful here.
>
>> More detailed explanation and background can be found in presentations
>> during LPC2022 [1][2] or in the documentation patches.
>
> I checked 15/17 as well but could find any of this information there either.
>
> [...]

For the above two comments: Yes, I see your point and will address that
in the next v3.

Thank you fro your valuable comments and time for reviewing it!

Lukasz