| 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: <CAPDyKFqsE4nArB+Qn5gH5P7vVePpZ6Z7ruas2-EGYsctGV-=4w@mail.gmail.com>
Date: Tue, 11 Mar 2025 15:42:31 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: rafael@...nel.org, Jacky Bai <ping.bai@....com>
Cc: daniel.lezcano@...aro.org, lpieralisi@...nel.org, sudeep.holla@....com,
james.morse@....com, d-gole@...com, anup@...infault.org,
paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu,
linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
imx@...ts.linux.dev, khilman@...libre.com, quic_tingweiz@...cinc.com,
quic_yuanjiey@...cinc.com
Subject: Re: [PATCH v5] cpuidle: Init cpuidle only for present CPUs
On Fri, 7 Mar 2025 at 15:54, Jacky Bai <ping.bai@....com> wrote:
>
> for_each_possible_cpu() is currently used to initialize cpuidle
> in below cpuidle drivers:
> drivers/cpuidle/cpuidle-arm.c
> drivers/cpuidle/cpuidle-big_little.c
> drivers/cpuidle/cpuidle-psci.c
> drivers/cpuidle/cpuidle-qcom-spm.c
> drivers/cpuidle/cpuidle-riscv-sbi.c
>
> However, in cpu_dev_register_generic(), for_each_present_cpu()
> is used to register CPU devices which means the CPU devices are
> only registered for present CPUs and not all possible CPUs.
>
> With nosmp or maxcpus=0, only the boot CPU is present, lead
> to the failure:
>
> | Failed to register cpuidle device for cpu1
>
> Then rollback to cancel all CPUs' cpuidle registration.
>
> Change for_each_possible_cpu() to for_each_present_cpu() in the
> above cpuidle drivers to ensure it only registers cpuidle devices
> for CPUs that are actually present.
>
> Fixes: b0c69e1214bc ("drivers: base: Use present CPUs in GENERIC_CPU_DEVICES")
> Reviewed-by: Dhruva Gole <d-gole@...com>
> Reviewed-by: Sudeep Holla <sudeep.holla@....com>
> Tested-by: Yuanjie Yang <quic_yuanjiey@...cinc.com>
> Signed-off-by: Jacky Bai <ping.bai@....com>
Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
Rafael, I assume you will pick these up? Or let me know if you want me
to take them via my pmdomain tree.
Kind regards
Uffe
> ---
> - v5 changes:
> - add changes for cpuidle-qcom-spm
>
> - v4 changes:
> - add changes for other cpuidle driver that has the similar issue
> as cpuidle-pcsi driver.
>
> - v3 changes:
> - improve the changelog as suggested by Sudeep
> ---
> drivers/cpuidle/cpuidle-arm.c | 8 ++++----
> drivers/cpuidle/cpuidle-big_little.c | 2 +-
> drivers/cpuidle/cpuidle-psci.c | 4 ++--
> drivers/cpuidle/cpuidle-qcom-spm.c | 2 +-
> drivers/cpuidle/cpuidle-riscv-sbi.c | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index caba6f4bb1b7..e044fefdb816 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -137,9 +137,9 @@ static int __init arm_idle_init_cpu(int cpu)
> /*
> * arm_idle_init - Initializes arm cpuidle driver
> *
> - * Initializes arm cpuidle driver for all CPUs, if any CPU fails
> - * to register cpuidle driver then rollback to cancel all CPUs
> - * registration.
> + * Initializes arm cpuidle driver for all present CPUs, if any
> + * CPU fails to register cpuidle driver then rollback to cancel
> + * all CPUs registration.
> */
> static int __init arm_idle_init(void)
> {
> @@ -147,7 +147,7 @@ static int __init arm_idle_init(void)
> struct cpuidle_driver *drv;
> struct cpuidle_device *dev;
>
> - for_each_possible_cpu(cpu) {
> + for_each_present_cpu(cpu) {
> ret = arm_idle_init_cpu(cpu);
> if (ret)
> goto out_fail;
> diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> index 74972deda0ea..4abba42fcc31 100644
> --- a/drivers/cpuidle/cpuidle-big_little.c
> +++ b/drivers/cpuidle/cpuidle-big_little.c
> @@ -148,7 +148,7 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int part_id)
> if (!cpumask)
> return -ENOMEM;
>
> - for_each_possible_cpu(cpu)
> + for_each_present_cpu(cpu)
> if (smp_cpuid_part(cpu) == part_id)
> cpumask_set_cpu(cpu, cpumask);
>
> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index dd8d776d6e39..b46a83f5ffe4 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c
> @@ -403,7 +403,7 @@ static int psci_idle_init_cpu(struct device *dev, int cpu)
> /*
> * psci_idle_probe - Initializes PSCI cpuidle driver
> *
> - * Initializes PSCI cpuidle driver for all CPUs, if any CPU fails
> + * Initializes PSCI cpuidle driver for all present CPUs, if any CPU fails
> * to register cpuidle driver then rollback to cancel all CPUs
> * registration.
> */
> @@ -413,7 +413,7 @@ static int psci_cpuidle_probe(struct platform_device *pdev)
> struct cpuidle_driver *drv;
> struct cpuidle_device *dev;
>
> - for_each_possible_cpu(cpu) {
> + for_each_present_cpu(cpu) {
> ret = psci_idle_init_cpu(&pdev->dev, cpu);
> if (ret)
> goto out_fail;
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 3ab240e0e122..5f386761b156 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -135,7 +135,7 @@ static int spm_cpuidle_drv_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(&pdev->dev, ret, "set warm boot addr failed");
>
> - for_each_possible_cpu(cpu) {
> + for_each_present_cpu(cpu) {
> ret = spm_cpuidle_register(&pdev->dev, cpu);
> if (ret && ret != -ENODEV) {
> dev_err(&pdev->dev,
> diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c
> index 0c92a628bbd4..0fe1ece9fbdc 100644
> --- a/drivers/cpuidle/cpuidle-riscv-sbi.c
> +++ b/drivers/cpuidle/cpuidle-riscv-sbi.c
> @@ -529,8 +529,8 @@ static int sbi_cpuidle_probe(struct platform_device *pdev)
> return ret;
> }
>
> - /* Initialize CPU idle driver for each CPU */
> - for_each_possible_cpu(cpu) {
> + /* Initialize CPU idle driver for each present CPU */
> + for_each_present_cpu(cpu) {
> ret = sbi_cpuidle_init_cpu(&pdev->dev, cpu);
> if (ret) {
> pr_debug("HART%ld: idle driver init failed\n",
> --
> 2.34.1
>
Powered by blists - more mailing lists