* [PATCH v1 0/3] PM: EM: API cleanups and elimination of __rcu-related sparse warnings
@ 2025-03-05 21:06 Rafael J. Wysocki
2025-03-05 21:08 ` [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain() Rafael J. Wysocki
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-03-05 21:06 UTC (permalink / raw)
To: Linux PM; +Cc: Lukasz Luba, LKML, Dietmar Eggemann, Ricardo Neri
Hi Everyone,
Patch [1/3] adds const to the cp and cpus parameters of
em_dev_register_perf_domain().
Patch [2/3] unexports functions that are only used locally in
the Energy Model code.
Patch [3/3] eliminates sparse warnings related to __rcu usage.
Please refer to the individual patch changelogs for details.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain()
2025-03-05 21:06 [PATCH v1 0/3] PM: EM: API cleanups and elimination of __rcu-related sparse warnings Rafael J. Wysocki
@ 2025-03-05 21:08 ` Rafael J. Wysocki
2025-03-06 9:22 ` Lukasz Luba
2025-03-05 21:11 ` [PATCH v1 2/3] PM: EM: Make three functions static Rafael J. Wysocki
2025-03-05 21:13 ` [PATCH v1 3/3] PM: EM: Address RCU-related sparse warnings Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-03-05 21:08 UTC (permalink / raw)
To: Linux PM; +Cc: Lukasz Luba, LKML, Dietmar Eggemann, Ricardo Neri
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Notice that em_dev_register_perf_domain() and the functions called by it
do not update objects pointed to by its cb and cpus parameters, so the
const modifier can be added to them.
This allows the return value of cpumask_of() or a pointer to a
struct em_data_callback declared as const to be passed to
em_dev_register_perf_domain() directly without explicit type
casting which is rather handy.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
include/linux/energy_model.h | 8 ++++----
kernel/power/energy_model.c | 11 ++++++-----
2 files changed, 10 insertions(+), 9 deletions(-)
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -169,8 +169,8 @@
int em_dev_update_perf_domain(struct device *dev,
struct em_perf_table __rcu *new_table);
int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
- struct em_data_callback *cb, cpumask_t *span,
- bool microwatts);
+ const struct em_data_callback *cb,
+ const cpumask_t *cpus, bool microwatts);
void em_dev_unregister_perf_domain(struct device *dev);
struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd);
void em_table_free(struct em_perf_table __rcu *table);
@@ -346,8 +346,8 @@
static inline
int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
- struct em_data_callback *cb, cpumask_t *span,
- bool microwatts)
+ const struct em_data_callback *cb,
+ const cpumask_t *cpus, bool microwatts)
{
return -EINVAL;
}
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -231,7 +231,7 @@
}
static int em_compute_costs(struct device *dev, struct em_perf_state *table,
- struct em_data_callback *cb, int nr_states,
+ const struct em_data_callback *cb, int nr_states,
unsigned long flags)
{
unsigned long prev_cost = ULONG_MAX;
@@ -333,7 +333,7 @@
static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
struct em_perf_state *table,
- struct em_data_callback *cb,
+ const struct em_data_callback *cb,
unsigned long flags)
{
unsigned long power, freq, prev_freq = 0;
@@ -388,7 +388,8 @@
}
static int em_create_pd(struct device *dev, int nr_states,
- struct em_data_callback *cb, cpumask_t *cpus,
+ const struct em_data_callback *cb,
+ const cpumask_t *cpus,
unsigned long flags)
{
struct em_perf_table __rcu *em_table;
@@ -548,8 +549,8 @@
* Return 0 on success
*/
int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
- struct em_data_callback *cb, cpumask_t *cpus,
- bool microwatts)
+ const struct em_data_callback *cb,
+ const cpumask_t *cpus, bool microwatts)
{
unsigned long cap, prev_cap = 0;
unsigned long flags = 0;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 2/3] PM: EM: Make three functions static
2025-03-05 21:06 [PATCH v1 0/3] PM: EM: API cleanups and elimination of __rcu-related sparse warnings Rafael J. Wysocki
2025-03-05 21:08 ` [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain() Rafael J. Wysocki
@ 2025-03-05 21:11 ` Rafael J. Wysocki
2025-03-06 10:01 ` Lukasz Luba
2025-03-05 21:13 ` [PATCH v1 3/3] PM: EM: Address RCU-related sparse warnings Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-03-05 21:11 UTC (permalink / raw)
To: Linux PM; +Cc: Lukasz Luba, LKML, Dietmar Eggemann, Ricardo Neri
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Three functions in the Energy Model code, em_dev_update_perf_domain(),
em_table_alloc() and em_table_free(), have no users outside that code and
so make them static, remove their headers from the Energy Model header
file and remove a piece of documentation associated with them.
This also helps to clean up RCU handling in the Energy Model code that
will be done subsequently.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
This essentially follow the rules that all functions without users in the
files where they are defined should be static (with all due respect to any
out-of-the-tree users of them).
This change can be reversed when any new users of these functions appear,
but it will have to take changes made by the subsequent patch into account.
---
Documentation/power/energy-model.rst | 39 +----------------------------------
include/linux/energy_model.h | 16 --------------
kernel/power/energy_model.c | 9 +++-----
3 files changed, 6 insertions(+), 58 deletions(-)
--- a/Documentation/power/energy-model.rst
+++ b/Documentation/power/energy-model.rst
@@ -203,43 +203,8 @@
or in Section 2.5
-2.4 Runtime modifications
-^^^^^^^^^^^^^^^^^^^^^^^^^
-
-Drivers willing to update the EM at runtime should use the following dedicated
-function to allocate a new instance of the modified EM. The API is listed
-below::
-
- struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd);
-
-This allows to allocate a structure which contains the new EM table with
-also RCU and kref needed by the EM framework. The 'struct em_perf_table'
-contains array 'struct em_perf_state state[]' which is a list of performance
-states in ascending order. That list must be populated by the device driver
-which wants to update the EM. The list of frequencies can be taken from
-existing EM (created during boot). The content in the 'struct em_perf_state'
-must be populated by the driver as well.
-
-This is the API which does the EM update, using RCU pointers swap::
-
- int em_dev_update_perf_domain(struct device *dev,
- struct em_perf_table __rcu *new_table);
-
-Drivers must provide a pointer to the allocated and initialized new EM
-'struct em_perf_table'. That new EM will be safely used inside the EM framework
-and will be visible to other sub-systems in the kernel (thermal, powercap).
-The main design goal for this API is to be fast and avoid extra calculations
-or memory allocations at runtime. When pre-computed EMs are available in the
-device driver, than it should be possible to simply re-use them with low
-performance overhead.
-
-In order to free the EM, provided earlier by the driver (e.g. when the module
-is unloaded), there is a need to call the API::
-
- void em_table_free(struct em_perf_table __rcu *table);
-
-It will allow the EM framework to safely remove the memory, when there is
-no other sub-system using it, e.g. EAS.
+2.4 Accessing power values and computing costs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
To use the power values in other sub-systems (like thermal, powercap) there is
a need to call API which protects the reader and provide consistency of the EM
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -166,14 +166,10 @@
struct em_perf_domain *em_cpu_get(int cpu);
struct em_perf_domain *em_pd_get(struct device *dev);
-int em_dev_update_perf_domain(struct device *dev,
- struct em_perf_table __rcu *new_table);
int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
const struct em_data_callback *cb,
const cpumask_t *cpus, bool microwatts);
void em_dev_unregister_perf_domain(struct device *dev);
-struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd);
-void em_table_free(struct em_perf_table __rcu *table);
int em_dev_compute_costs(struct device *dev, struct em_perf_state *table,
int nr_states);
int em_dev_update_chip_binning(struct device *dev);
@@ -374,18 +370,6 @@
return 0;
}
static inline
-struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd)
-{
- return NULL;
-}
-static inline void em_table_free(struct em_perf_table __rcu *table) {}
-static inline
-int em_dev_update_perf_domain(struct device *dev,
- struct em_perf_table __rcu *new_table)
-{
- return -EINVAL;
-}
-static inline
struct em_perf_state *em_perf_state_from_pd(struct em_perf_domain *pd)
{
return NULL;
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -177,7 +177,7 @@
*
* No return values.
*/
-void em_table_free(struct em_perf_table __rcu *table)
+static void em_table_free(struct em_perf_table __rcu *table)
{
kref_put(&table->kref, em_release_table_kref);
}
@@ -190,7 +190,7 @@
* has a user.
* Returns allocated table or NULL.
*/
-struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd)
+static struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd)
{
struct em_perf_table __rcu *table;
int table_size;
@@ -299,8 +299,8 @@
*
* Return 0 on success or an error code on failure.
*/
-int em_dev_update_perf_domain(struct device *dev,
- struct em_perf_table __rcu *new_table)
+static int em_dev_update_perf_domain(struct device *dev,
+ struct em_perf_table __rcu *new_table)
{
struct em_perf_table __rcu *old_table;
struct em_perf_domain *pd;
@@ -329,7 +329,6 @@
mutex_unlock(&em_pd_mutex);
return 0;
}
-EXPORT_SYMBOL_GPL(em_dev_update_perf_domain);
static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
struct em_perf_state *table,
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 3/3] PM: EM: Address RCU-related sparse warnings
2025-03-05 21:06 [PATCH v1 0/3] PM: EM: API cleanups and elimination of __rcu-related sparse warnings Rafael J. Wysocki
2025-03-05 21:08 ` [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain() Rafael J. Wysocki
2025-03-05 21:11 ` [PATCH v1 2/3] PM: EM: Make three functions static Rafael J. Wysocki
@ 2025-03-05 21:13 ` Rafael J. Wysocki
2 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-03-05 21:13 UTC (permalink / raw)
To: Linux PM; +Cc: Lukasz Luba, LKML, Dietmar Eggemann, Ricardo Neri
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The usage of __rcu in the Energy Model code is quite inconsistent
which causes the following sparse warnings to trigger:
kernel/power/energy_model.c:169:15: warning: incorrect type in assignment (different address spaces)
kernel/power/energy_model.c:169:15: expected struct em_perf_table [noderef] __rcu *table
kernel/power/energy_model.c:169:15: got struct em_perf_table *
kernel/power/energy_model.c:171:9: warning: incorrect type in argument 1 (different address spaces)
kernel/power/energy_model.c:171:9: expected struct callback_head *head
kernel/power/energy_model.c:171:9: got struct callback_head [noderef] __rcu *
kernel/power/energy_model.c:171:9: warning: cast removes address space '__rcu' of expression
kernel/power/energy_model.c:182:19: warning: incorrect type in argument 1 (different address spaces)
kernel/power/energy_model.c:182:19: expected struct kref *kref
kernel/power/energy_model.c:182:19: got struct kref [noderef] __rcu *
kernel/power/energy_model.c:200:15: warning: incorrect type in assignment (different address spaces)
kernel/power/energy_model.c:200:15: expected struct em_perf_table [noderef] __rcu *table
kernel/power/energy_model.c:200:15: got void *[assigned] _res
kernel/power/energy_model.c:204:20: warning: incorrect type in argument 1 (different address spaces)
kernel/power/energy_model.c:204:20: expected struct kref *kref
kernel/power/energy_model.c:204:20: got struct kref [noderef] __rcu *
kernel/power/energy_model.c:320:19: warning: incorrect type in argument 1 (different address spaces)
kernel/power/energy_model.c:320:19: expected struct kref *kref
kernel/power/energy_model.c:320:19: got struct kref [noderef] __rcu *
kernel/power/energy_model.c:325:45: warning: incorrect type in argument 2 (different address spaces)
kernel/power/energy_model.c:325:45: expected struct em_perf_state *table
kernel/power/energy_model.c:325:45: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:425:45: warning: incorrect type in argument 3 (different address spaces)
kernel/power/energy_model.c:425:45: expected struct em_perf_state *table
kernel/power/energy_model.c:425:45: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:442:15: warning: incorrect type in argument 1 (different address spaces)
kernel/power/energy_model.c:442:15: expected void const *objp
kernel/power/energy_model.c:442:15: got struct em_perf_table [noderef] __rcu *[assigned] em_table
kernel/power/energy_model.c:626:55: warning: incorrect type in argument 2 (different address spaces)
kernel/power/energy_model.c:626:55: expected struct em_perf_state *table
kernel/power/energy_model.c:626:55: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:681:16: warning: incorrect type in assignment (different address spaces)
kernel/power/energy_model.c:681:16: expected struct em_perf_state *new_ps
kernel/power/energy_model.c:681:16: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:699:37: warning: incorrect type in argument 2 (different address spaces)
kernel/power/energy_model.c:699:37: expected struct em_perf_state *table
kernel/power/energy_model.c:699:37: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:733:38: warning: incorrect type in argument 3 (different address spaces)
kernel/power/energy_model.c:733:38: expected struct em_perf_state *table
kernel/power/energy_model.c:733:38: got struct em_perf_state [noderef] __rcu *
kernel/power/energy_model.c:855:53: warning: dereference of noderef expression
kernel/power/energy_model.c:864:32: warning: dereference of noderef expression
This is because the __rcu annotation for sparse is only applicable to
pointers that need rcu_dereference() or equivalent for protection, which
basically means pointers assigned with rcu_assign_pointer().
Make all of the above sparse warnings go away by cleaning up the usage
of __rcu and using rcu_dereference_protected() where applicable.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
kernel/power/energy_model.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -163,12 +163,8 @@
static void em_release_table_kref(struct kref *kref)
{
- struct em_perf_table __rcu *table;
-
/* It was the last owner of this table so we can free */
- table = container_of(kref, struct em_perf_table, kref);
-
- kfree_rcu(table, rcu);
+ kfree_rcu(container_of(kref, struct em_perf_table, kref), rcu);
}
/**
@@ -177,7 +173,7 @@
*
* No return values.
*/
-static void em_table_free(struct em_perf_table __rcu *table)
+static void em_table_free(struct em_perf_table *table)
{
kref_put(&table->kref, em_release_table_kref);
}
@@ -190,9 +186,9 @@
* has a user.
* Returns allocated table or NULL.
*/
-static struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd)
+static struct em_perf_table *em_table_alloc(struct em_perf_domain *pd)
{
- struct em_perf_table __rcu *table;
+ struct em_perf_table *table;
int table_size;
table_size = sizeof(struct em_perf_state) * pd->nr_perf_states;
@@ -300,9 +296,9 @@
* Return 0 on success or an error code on failure.
*/
static int em_dev_update_perf_domain(struct device *dev,
- struct em_perf_table __rcu *new_table)
+ struct em_perf_table *new_table)
{
- struct em_perf_table __rcu *old_table;
+ struct em_perf_table *old_table;
struct em_perf_domain *pd;
if (!dev)
@@ -319,7 +315,8 @@
kref_get(&new_table->kref);
- old_table = pd->em_table;
+ old_table = rcu_dereference_protected(pd->em_table,
+ lockdep_is_held(&em_pd_mutex));
rcu_assign_pointer(pd->em_table, new_table);
em_cpufreq_update_efficiencies(dev, new_table->state);
@@ -391,7 +388,7 @@
const cpumask_t *cpus,
unsigned long flags)
{
- struct em_perf_table __rcu *em_table;
+ struct em_perf_table *em_table;
struct em_perf_domain *pd;
struct device *cpu_dev;
int cpu, ret, num_cpus;
@@ -551,6 +548,7 @@
const struct em_data_callback *cb,
const cpumask_t *cpus, bool microwatts)
{
+ struct em_perf_table *em_table;
unsigned long cap, prev_cap = 0;
unsigned long flags = 0;
int cpu, ret;
@@ -623,7 +621,9 @@
dev->em_pd->min_perf_state = 0;
dev->em_pd->max_perf_state = nr_states - 1;
- em_cpufreq_update_efficiencies(dev, dev->em_pd->em_table->state);
+ em_table = rcu_dereference_protected(dev->em_pd->em_table,
+ lockdep_is_held(&em_pd_mutex));
+ em_cpufreq_update_efficiencies(dev, em_table->state);
em_debug_create_pd(dev);
dev_info(dev, "EM: created perf domain\n");
@@ -660,7 +660,8 @@
mutex_lock(&em_pd_mutex);
em_debug_remove_pd(dev);
- em_table_free(dev->em_pd->em_table);
+ em_table_free(rcu_dereference_protected(dev->em_pd->em_table,
+ lockdep_is_held(&em_pd_mutex)));
kfree(dev->em_pd);
dev->em_pd = NULL;
@@ -668,9 +669,9 @@
}
EXPORT_SYMBOL_GPL(em_dev_unregister_perf_domain);
-static struct em_perf_table __rcu *em_table_dup(struct em_perf_domain *pd)
+static struct em_perf_table *em_table_dup(struct em_perf_domain *pd)
{
- struct em_perf_table __rcu *em_table;
+ struct em_perf_table *em_table;
struct em_perf_state *ps, *new_ps;
int ps_size;
@@ -692,7 +693,7 @@
}
static int em_recalc_and_update(struct device *dev, struct em_perf_domain *pd,
- struct em_perf_table __rcu *em_table)
+ struct em_perf_table *em_table)
{
int ret;
@@ -722,7 +723,7 @@
static void em_adjust_new_capacity(struct device *dev,
struct em_perf_domain *pd)
{
- struct em_perf_table __rcu *em_table;
+ struct em_perf_table *em_table;
em_table = em_table_dup(pd);
if (!em_table) {
@@ -831,7 +832,7 @@
*/
int em_dev_update_chip_binning(struct device *dev)
{
- struct em_perf_table __rcu *em_table;
+ struct em_perf_table *em_table;
struct em_perf_domain *pd;
int i, ret;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain()
2025-03-05 21:08 ` [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain() Rafael J. Wysocki
@ 2025-03-06 9:22 ` Lukasz Luba
0 siblings, 0 replies; 7+ messages in thread
From: Lukasz Luba @ 2025-03-06 9:22 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: LKML, Dietmar Eggemann, Ricardo Neri, Linux PM
On 3/5/25 21:08, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Notice that em_dev_register_perf_domain() and the functions called by it
> do not update objects pointed to by its cb and cpus parameters, so the
> const modifier can be added to them.
>
> This allows the return value of cpumask_of() or a pointer to a
> struct em_data_callback declared as const to be passed to
> em_dev_register_perf_domain() directly without explicit type
> casting which is rather handy.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> include/linux/energy_model.h | 8 ++++----
> kernel/power/energy_model.c | 11 ++++++-----
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -169,8 +169,8 @@
> int em_dev_update_perf_domain(struct device *dev,
> struct em_perf_table __rcu *new_table);
> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
> - struct em_data_callback *cb, cpumask_t *span,
> - bool microwatts);
> + const struct em_data_callback *cb,
> + const cpumask_t *cpus, bool microwatts);
> void em_dev_unregister_perf_domain(struct device *dev);
> struct em_perf_table __rcu *em_table_alloc(struct em_perf_domain *pd);
> void em_table_free(struct em_perf_table __rcu *table);
> @@ -346,8 +346,8 @@
>
> static inline
> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
> - struct em_data_callback *cb, cpumask_t *span,
> - bool microwatts)
> + const struct em_data_callback *cb,
> + const cpumask_t *cpus, bool microwatts)
> {
> return -EINVAL;
> }
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -231,7 +231,7 @@
> }
>
> static int em_compute_costs(struct device *dev, struct em_perf_state *table,
> - struct em_data_callback *cb, int nr_states,
> + const struct em_data_callback *cb, int nr_states,
> unsigned long flags)
> {
> unsigned long prev_cost = ULONG_MAX;
> @@ -333,7 +333,7 @@
>
> static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
> struct em_perf_state *table,
> - struct em_data_callback *cb,
> + const struct em_data_callback *cb,
> unsigned long flags)
> {
> unsigned long power, freq, prev_freq = 0;
> @@ -388,7 +388,8 @@
> }
>
> static int em_create_pd(struct device *dev, int nr_states,
> - struct em_data_callback *cb, cpumask_t *cpus,
> + const struct em_data_callback *cb,
> + const cpumask_t *cpus,
> unsigned long flags)
> {
> struct em_perf_table __rcu *em_table;
> @@ -548,8 +549,8 @@
> * Return 0 on success
> */
> int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
> - struct em_data_callback *cb, cpumask_t *cpus,
> - bool microwatts)
> + const struct em_data_callback *cb,
> + const cpumask_t *cpus, bool microwatts)
> {
> unsigned long cap, prev_cap = 0;
> unsigned long flags = 0;
>
>
>
LGTM,
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/3] PM: EM: Make three functions static
2025-03-05 21:11 ` [PATCH v1 2/3] PM: EM: Make three functions static Rafael J. Wysocki
@ 2025-03-06 10:01 ` Lukasz Luba
2025-03-06 10:37 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Lukasz Luba @ 2025-03-06 10:01 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: LKML, Linux PM, Dietmar Eggemann, Ricardo Neri
Hi Rafael,
On 3/5/25 21:11, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Three functions in the Energy Model code, em_dev_update_perf_domain(),
> em_table_alloc() and em_table_free(), have no users outside that code and
> so make them static, remove their headers from the Energy Model header
> file and remove a piece of documentation associated with them.
>
> This also helps to clean up RCU handling in the Energy Model code that
> will be done subsequently.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> This essentially follow the rules that all functions without users in the
> files where they are defined should be static (with all due respect to any
> out-of-the-tree users of them).
>
> This change can be reversed when any new users of these functions appear,
> but it will have to take changes made by the subsequent patch into account.
>
I see your point and it's valid.
Although, please give me a few days and I will send some patches which
add a client for this API. It will be a modification of the EM for
CPUs while the GPU is producing heat to the SoC. Then IPA and EAS
will get the updated total power values (doe to this this leakage power)
in the EM.
As of now, I had some code downstream for research, that I share with
partners in the Android world [1].
I believe the user-space sysfs (like in that top patch) which allows
such EM modification would not be accepted?
Such approach might also help the Middle-ware in the OS to influence the
kernel decisions, mainly on phones, where the app just occupies the
screen and Middle-ware knows about it.
Regards,
Lukasz
[1]
https://gitlab.arm.com/linux-arm/linux-power/-/commits/dynamic_energy_model/android14-v6.1/v6.1.75/?ref_type=heads
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/3] PM: EM: Make three functions static
2025-03-06 10:01 ` Lukasz Luba
@ 2025-03-06 10:37 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-03-06 10:37 UTC (permalink / raw)
To: Lukasz Luba
Cc: Rafael J. Wysocki, LKML, Linux PM, Dietmar Eggemann, Ricardo Neri
On Thu, Mar 6, 2025 at 11:01 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Rafael,
>
> On 3/5/25 21:11, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Three functions in the Energy Model code, em_dev_update_perf_domain(),
> > em_table_alloc() and em_table_free(), have no users outside that code and
> > so make them static, remove their headers from the Energy Model header
> > file and remove a piece of documentation associated with them.
> >
> > This also helps to clean up RCU handling in the Energy Model code that
> > will be done subsequently.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > This essentially follow the rules that all functions without users in the
> > files where they are defined should be static (with all due respect to any
> > out-of-the-tree users of them).
> >
> > This change can be reversed when any new users of these functions appear,
> > but it will have to take changes made by the subsequent patch into account.
> >
>
> I see your point and it's valid.
>
> Although, please give me a few days and I will send some patches which
> add a client for this API. It will be a modification of the EM for
> CPUs while the GPU is producing heat to the SoC. Then IPA and EAS
> will get the updated total power values (doe to this this leakage power)
> in the EM.
OK, so I'll need to change the headers in the next patch and I'll drop this one.
> As of now, I had some code downstream for research, that I share with
> partners in the Android world [1].
> I believe the user-space sysfs (like in that top patch) which allows
> such EM modification would not be accepted?
Well, if there's a good enough reason for its existence, then it can
be added I think. It all depends on how this is expected to be used.
> Such approach might also help the Middle-ware in the OS to influence the
> kernel decisions, mainly on phones, where the app just occupies the
> screen and Middle-ware knows about it.
You need to be cautious about changing the EM too often though as that
would only lead to thrashing and nothing beneficial.
> [1]
> https://gitlab.arm.com/linux-arm/linux-power/-/commits/dynamic_energy_model/android14-v6.1/v6.1.75/?ref_type=heads
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-06 10:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 21:06 [PATCH v1 0/3] PM: EM: API cleanups and elimination of __rcu-related sparse warnings Rafael J. Wysocki
2025-03-05 21:08 ` [PATCH v1 1/3] PM: EM: Consify two parameters of em_dev_register_perf_domain() Rafael J. Wysocki
2025-03-06 9:22 ` Lukasz Luba
2025-03-05 21:11 ` [PATCH v1 2/3] PM: EM: Make three functions static Rafael J. Wysocki
2025-03-06 10:01 ` Lukasz Luba
2025-03-06 10:37 ` Rafael J. Wysocki
2025-03-05 21:13 ` [PATCH v1 3/3] PM: EM: Address RCU-related sparse warnings Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox