| 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: <CAJZ5v0jckgZfuh=yAqoftG1Q-1z0ngLXQa4TX-iwuy54UmWMng@mail.gmail.com>
Date: Sat, 23 Aug 2025 18:13:44 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Huisong Li <lihuisong@...wei.com>
Cc: rafael@...nel.org, lenb@...nel.org, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, linuxarm@...wei.com,
jonathan.cameron@...wei.com, zhanjie9@...ilicon.com, zhenglifeng1@...wei.com,
yubowen8@...wei.com, liuyonglong@...wei.com
Subject: Re: [PATCH v3 2/2] ACPI: processor: idle: Optimize acpi idle driver registration
On Mon, Jul 28, 2025 at 9:06 AM Huisong Li <lihuisong@...wei.com> wrote:
>
> Currently, the acpi idle driver is registered from within a CPU
> hotplug callback. Although this didn't cause any functional issues,
> this is questionable and confusing. And it is better to register
> the cpuidle driver when all of the CPUs have been brought up.
>
> So add a new function to initialize acpi_idle_driver based on the
> power management information of an available CPU and register cpuidle
> driver in acpi_processor_driver_init().
>
> Signed-off-by: Huisong Li <lihuisong@...wei.com>
> ---
> drivers/acpi/processor_driver.c | 3 ++
> drivers/acpi/processor_idle.c | 65 +++++++++++++++++++++------------
> include/acpi/processor.h | 8 ++++
> 3 files changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 65e779be64ff..bc9f58a02c1d 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -263,6 +263,8 @@ static int __init acpi_processor_driver_init(void)
> if (result < 0)
> return result;
>
> + acpi_processor_register_idle_driver();
> +
> result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> "acpi/cpu-drv:online",
> acpi_soft_cpu_online, NULL);
> @@ -301,6 +303,7 @@ static void __exit acpi_processor_driver_exit(void)
>
> cpuhp_remove_state_nocalls(hp_online);
> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> + acpi_processor_unregister_idle_driver();
> driver_unregister(&acpi_processor_driver);
> }
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 031738390f2d..c71802d42e8a 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1360,7 +1360,48 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
> return 0;
> }
>
> -static int acpi_processor_registered;
> +void acpi_processor_register_idle_driver(void)
> +{
> + struct acpi_processor *pr;
> + int ret = -ENODEV;
> + int cpu;
> +
> + /*
> + * Acpi idle driver is used by all possible CPUs.
> + * Install the idle handler by the processor power info of one in them.
> + * Note that we use previously set idle handler will be used on
> + * platforms that only support C1.
> + */
> + for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
> + pr = per_cpu(processors, cpu);
> + if (!pr)
> + continue;
> +
> + ret = acpi_processor_get_power_info(pr);
> + if (!ret) {
> + pr->flags.power_setup_done = 1;
> + acpi_processor_setup_cpuidle_states(pr);
> + break;
> + }
> + }
> +
> + if (ret) {
> + pr_debug("No ACPI power information from any CPUs.\n");
> + return;
> + }
> +
> + ret = cpuidle_register_driver(&acpi_idle_driver);
> + if (ret) {
> + pr_debug("register %s failed.\n", acpi_idle_driver.name);
> + return;
> + }
> + pr_debug("%s registered with cpuidle.\n", acpi_idle_driver.name);
> +}
> +
> +void acpi_processor_unregister_idle_driver(void)
> +{
> + cpuidle_unregister_driver(&acpi_idle_driver);
> +}
>
> int acpi_processor_power_init(struct acpi_processor *pr)
> {
> @@ -1375,22 +1416,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> if (!acpi_processor_get_power_info(pr))
> pr->flags.power_setup_done = 1;
>
> - /*
> - * Install the idle handler if processor power management is supported.
> - * Note that we use previously set idle handler will be used on
> - * platforms that only support C1.
> - */
> if (pr->flags.power) {
> - /* Register acpi_idle_driver if not already registered */
> - if (!acpi_processor_registered) {
> - acpi_processor_setup_cpuidle_states(pr);
> - retval = cpuidle_register_driver(&acpi_idle_driver);
> - if (retval)
> - return retval;
> - pr_debug("%s registered with cpuidle\n",
> - acpi_idle_driver.name);
> - }
> -
> dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> if (!dev)
> return -ENOMEM;
> @@ -1403,13 +1429,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> */
> retval = cpuidle_register_device(dev);
> if (retval) {
> - if (acpi_processor_registered == 0)
> - cpuidle_unregister_driver(&acpi_idle_driver);
> kfree(dev);
> per_cpu(acpi_cpuidle_device, pr->id) = NULL;
> return retval;
> }
> - acpi_processor_registered++;
> }
> return 0;
> }
> @@ -1423,10 +1446,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
>
> if (pr->flags.power) {
> cpuidle_unregister_device(dev);
> - acpi_processor_registered--;
> - if (acpi_processor_registered == 0)
> - cpuidle_unregister_driver(&acpi_idle_driver);
> -
> kfree(dev);
> }
>
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index d0eccbd920e5..1249f5e81d92 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
> int acpi_processor_power_exit(struct acpi_processor *pr);
> int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
> int acpi_processor_hotplug(struct acpi_processor *pr);
> +void acpi_processor_register_idle_driver(void);
> +void acpi_processor_unregister_idle_driver(void);
> #else
> static inline int acpi_processor_power_init(struct acpi_processor *pr)
> {
> @@ -443,6 +445,12 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> return -ENODEV;
> }
> +static void acpi_processor_register_idle_driver(void)
> +{
> +}
> +static void acpi_processor_unregister_idle_driver(void)
> +{
> +}
> #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
>
> /* in processor_thermal.c */
> --
Applied as 6.18 material, thanks!
While at it, in the future, please always spell ACPI in capitals in
patch subjects, changelogs and code comments.
Powered by blists - more mailing lists