linux-kernel - Re: [PATCH v8 01/16] ACPI: processor: Simplify initial onlining to use same path for cold and hotplug

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6347020E-CB49-44ED-87B2-3BB2AA2F59E0@oracle.com>
Date: Fri, 26 Apr 2024 16:05:16 +0000
From: Miguel Luis <miguel.luis@...cle.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra
	<peterz@...radead.org>,
        "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>,
        "loongarch@...ts.linux.dev"
	<loongarch@...ts.linux.dev>,
        "linux-acpi@...r.kernel.org"
	<linux-acpi@...r.kernel.org>,
        "linux-arch@...r.kernel.org"
	<linux-arch@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>,
        "x86@...nel.org" <x86@...nel.org>,
        Russell King
	<linux@...linux.org.uk>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        James
 Morse <james.morse@....com>,
        Salil Mehta <salil.mehta@...wei.com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Catalin Marinas
	<catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Marc Zyngier
	<maz@...nel.org>,
        Hanjun Guo <guohanjun@...wei.com>, Ingo Molnar
	<mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen
	<dave.hansen@...ux.intel.com>,
        "linuxarm@...wei.com" <linuxarm@...wei.com>,
        "justin.he@....com" <justin.he@....com>,
        "jianyong.wu@....com"
	<jianyong.wu@....com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Sudeep
 Holla <sudeep.holla@....com>
Subject: Re: [PATCH v8 01/16] ACPI: processor: Simplify initial onlining to
 use same path for cold and hotplug



> On 26 Apr 2024, at 13:51, Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:
> 
> Separate code paths, combined with a flag set in acpi_processor.c to
> indicate a struct acpi_processor was for a hotplugged CPU ensured that
> per CPU data was only set up the first time that a CPU was initialized.
> This appears to be unnecessary as the paths can be combined by letting
> the online logic also handle any CPUs online at the time of driver load.
> 
> Motivation for this change, beyond simplification, is that ARM64
> virtual CPU HP uses the same code paths for hotplug and cold path in
> acpi_processor.c so had no easy way to set the flag for hotplug only.
> Removing this necessity will enable ARM64 vCPU HP to reuse the existing
> code paths.
> 
> Leave noisy pr_info() in place but update it to not state the CPU
> was hotplugged.
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Reviewed-by: Hanjun Guo <guohanjun@...wei.com>
> Tested-by: Miguel Luis <miguel.luis@...cle.com>
> Reviewed-by: Gavin Shan <gshan@...hat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> 
> ---
> v8: No change
> ---
> drivers/acpi/acpi_processor.c   |  1 -
> drivers/acpi/processor_driver.c | 44 ++++++++++-----------------------
> include/acpi/processor.h        |  2 +-
> 3 files changed, 14 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 7a0dd35d62c9..7fc924aeeed0 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -216,7 +216,6 @@ static int acpi_processor_hotadd_init(struct acpi_processor *pr)
> * gets online for the first time.
> */
> pr_info("CPU%d has been hot-added\n", pr->id);
> - pr->flags.need_hotplug_init = 1;
> 
> out:
> cpus_write_unlock();
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 67db60eda370..55782eac3ff1 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -33,7 +33,6 @@ MODULE_AUTHOR("Paul Diefenbaugh");
> MODULE_DESCRIPTION("ACPI Processor Driver");
> MODULE_LICENSE("GPL");
> 
> -static int acpi_processor_start(struct device *dev);
> static int acpi_processor_stop(struct device *dev);
> 
> static const struct acpi_device_id processor_device_ids[] = {
> @@ -47,7 +46,6 @@ static struct device_driver acpi_processor_driver = {
> .name = "processor",
> .bus = &cpu_subsys,
> .acpi_match_table = processor_device_ids,
> - .probe = acpi_processor_start,
> .remove = acpi_processor_stop,
> };
> 
> @@ -115,12 +113,10 @@ static int acpi_soft_cpu_online(unsigned int cpu)
> * CPU got physically hotplugged and onlined for the first time:
> * Initialize missing things.
> */
> - if (pr->flags.need_hotplug_init) {
> + if (!pr->flags.previously_online) {
> int ret;
> 
> - pr_info("Will online and init hotplugged CPU: %d\n",
> - pr->id);
> - pr->flags.need_hotplug_init = 0;
> + pr_info("Will online and init CPU: %d\n", pr->id);
> ret = __acpi_processor_start(device);
> WARN(ret, "Failed to start CPU: %d\n", pr->id);
> } else {
> @@ -167,9 +163,6 @@ static int __acpi_processor_start(struct acpi_device *device)
> if (!pr)
> return -ENODEV;
> 
> - if (pr->flags.need_hotplug_init)
> - return 0;
> -
> result = acpi_cppc_processor_probe(pr);
> if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> @@ -185,32 +178,21 @@ static int __acpi_processor_start(struct acpi_device *device)
> 
> status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
>     acpi_processor_notify, device);
> - if (ACPI_SUCCESS(status))
> - return 0;
> + if (!ACPI_SUCCESS(status)) {
> + result = -ENODEV;
> + goto err_thermal_exit;
> + }
> + pr->flags.previously_online = 1;
> 
> - result = -ENODEV;
> - acpi_processor_thermal_exit(pr, device);
> + return 0;
> 
> +err_thermal_exit:
> + acpi_processor_thermal_exit(pr, device);
> err_power_exit:
> acpi_processor_power_exit(pr);
> return result;
> }
> 
> -static int acpi_processor_start(struct device *dev)
> -{
> - struct acpi_device *device = ACPI_COMPANION(dev);
> - int ret;
> -
> - if (!device)
> - return -ENODEV;
> -
> - /* Protect against concurrent CPU hotplug operations */
> - cpu_hotplug_disable();
> - ret = __acpi_processor_start(device);
> - cpu_hotplug_enable();
> - return ret;
> -}
> -
> static int acpi_processor_stop(struct device *dev)
> {
> struct acpi_device *device = ACPI_COMPANION(dev);
> @@ -279,9 +261,9 @@ static int __init acpi_processor_driver_init(void)
> if (result < 0)
> return result;
> 
> - result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> -   "acpi/cpu-drv:online",
> -   acpi_soft_cpu_online, NULL);
> + result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +   "acpi/cpu-drv:online",
> +   acpi_soft_cpu_online, NULL);
> if (result < 0)
> goto err;
> hp_online = result;
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 3f34ebb27525..e6f6074eadbf 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -217,7 +217,7 @@ struct acpi_processor_flags {
> u8 has_lpi:1;
> u8 power_setup_done:1;
> u8 bm_rld_set:1;
> - u8 need_hotplug_init:1;
> + u8 previously_online:1;

Reviewed-by: Miguel Luis <miguel.luis@...cle.com>

Miguel

> };
> 
> struct acpi_processor {
> -- 
> 2.39.2
>