| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0inBsg+g150_tFfkff9bJVRE2XCRY-zDcDpfNvAcmHsCQ@mail.gmail.com>
Date: Wed, 10 Sep 2025 12:00:54 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Linux PM <linux-pm@...r.kernel.org>, Lukasz Luba <lukasz.luba@....com>,
LKML <linux-kernel@...r.kernel.org>, Dietmar Eggemann <dietmar.eggemann@....com>,
Christian Loehle <christian.loehle@....com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, "Kenneth R. Crudup" <kenny@...ix.com>
Subject: Re: [PATCH v1] PM: EM: Add function for registering a PD without
capacity update
On Fri, Sep 5, 2025 at 3:44 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> The intel_pstate driver manages CPU capacity changes itself and it does
> not need an update of the capacity of all CPUs in the system to be
> carried out after registering a PD.
>
> Moreover, in some configurations (for instance, an SMT-capable
> hybrid x86 system booted with nosmt in the kernel command line) the
> em_check_capacity_update() call at the end of em_dev_register_perf_domain()
> always fails and reschedules itself to run once again in 1 s, so
> effectively it runs in vain every 1 s forever.
>
> To address this, introduce a new variant of em_dev_register_perf_domain(),
> called em_dev_register_pd_no_update(), that does not invoke
> em_check_capacity_update(), and make intel_pstate use it instead of the
> original.
>
> Fixes: 7b010f9b9061 ("cpufreq: intel_pstate: EAS support for hybrid platforms")
> Link: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
> Reported-by: Kenneth R. Crudup <kenny@...ix.com>
> Tested-by: Kenneth R. Crudup <kenny@...ix.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Cc: 6.16+ <stable@...r.kernel.org> # 6.16+
I'd appreciate feedback on this.
Since it is 6.16-stable material, I'm inclined to just apply it and
let it into the mainline.
> ---
>
> Normally, I would place the definition of em_dev_register_pd_no_update()
> before the definition of em_dev_register_perf_domain() which called it,
> but in this particular case that would cause the real change to be much
> harder to grasp.
>
> ---
> drivers/cpufreq/intel_pstate.c | 4 ++--
> include/linux/energy_model.h | 10 ++++++++++
> kernel/power/energy_model.c | 29 +++++++++++++++++++++++++----
> 3 files changed, 37 insertions(+), 6 deletions(-)
>
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1034,8 +1034,8 @@ static bool hybrid_register_perf_domain(
> if (!cpu_dev)
> return false;
>
> - if (em_dev_register_perf_domain(cpu_dev, HYBRID_EM_STATE_COUNT, &cb,
> - cpumask_of(cpu), false))
> + if (em_dev_register_pd_no_update(cpu_dev, HYBRID_EM_STATE_COUNT, &cb,
> + cpumask_of(cpu), false))
> return false;
>
> cpudata->pd_registered = true;
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -171,6 +171,9 @@ int em_dev_update_perf_domain(struct dev
> 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);
> +int em_dev_register_pd_no_update(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 *em_table_alloc(struct em_perf_domain *pd);
> void em_table_free(struct em_perf_table *table);
> @@ -350,6 +353,13 @@ int em_dev_register_perf_domain(struct d
> {
> return -EINVAL;
> }
> +static inline
> +int em_dev_register_pd_no_update(struct device *dev, unsigned int nr_states,
> + const struct em_data_callback *cb,
> + const cpumask_t *cpus, bool microwatts)
> +{
> + return -EINVAL;
> +}
> static inline void em_dev_unregister_perf_domain(struct device *dev)
> {
> }
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -553,6 +553,30 @@ int em_dev_register_perf_domain(struct d
> const struct em_data_callback *cb,
> const cpumask_t *cpus, bool microwatts)
> {
> + int ret = em_dev_register_pd_no_update(dev, nr_states, cb, cpus, microwatts);
> +
> + if (_is_cpu_device(dev))
> + em_check_capacity_update();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(em_dev_register_perf_domain);
> +
> +/**
> + * em_dev_register_pd_no_update() - Register a perf domain for a device
> + * @dev : Device to register the PD for
> + * @nr_states : Number of performance states in the new PD
> + * @cb : Callback functions for populating the energy model
> + * @cpus : CPUs to include in the new PD (mandatory if @dev is a CPU device)
> + * @microwatts : Whether or not the power values in the EM will be in uW
> + *
> + * Like em_dev_register_perf_domain(), but does not trigger a CPU capacity
> + * update after registering the PD, even if @dev is a CPU device.
> + */
> +int em_dev_register_pd_no_update(struct device *dev, unsigned int nr_states,
> + 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;
> @@ -636,12 +660,9 @@ int em_dev_register_perf_domain(struct d
> unlock:
> mutex_unlock(&em_pd_mutex);
>
> - if (_is_cpu_device(dev))
> - em_check_capacity_update();
> -
> return ret;
> }
> -EXPORT_SYMBOL_GPL(em_dev_register_perf_domain);
> +EXPORT_SYMBOL_GPL(em_dev_register_pd_no_update);
>
> /**
> * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
>
>
>
>
Powered by blists - more mailing lists