linux-kernel - Re: [PATCH v5 03/18] ACPI: processor: Register deferred CPUs from acpi_processor_get

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]

Date: Tue, 16 Apr 2024 21:02:02 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, 
	"Russell King (Oracle)" <linux@...linux.org.uk>, linux-pm@...r.kernel.org, loongarch@...ts.linux.dev, 
	linux-acpi@...r.kernel.org, linux-arch@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	kvmarm@...ts.linux.dev, x86@...nel.org, Miguel Luis <miguel.luis@...cle.com>, 
	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>, linuxarm@...wei.com, justin.he@....com, 
	jianyong.wu@....com
Subject: Re: [PATCH v5 03/18] ACPI: processor: Register deferred CPUs from acpi_processor_get_info()

On Tue, Apr 16, 2024 at 7:41 PM Jonathan Cameron
<Jonathan.Cameron@...wei.com> wrote:
>
> On Mon, 15 Apr 2024 13:23:51 +0100
> Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:
>
> > On Mon, 15 Apr 2024 14:04:26 +0200
> > "Rafael J. Wysocki" <rafael@...nel.org> wrote:

[cut]

> > > > I'm still very much stuck on the hotadd_init flag however, so any suggestions
> > > > on that would be very welcome!
> > >
> > > I need to do some investigation which will take some time I suppose.
> >
> > I'll do so as well once I've gotten the rest sorted out.  That whole
> > structure seems overly complex and liable to race, though maybe sufficient
> > locking happens to be held that it's not a problem.
>
> Back to this a (maybe) last outstanding problem.
>
> Superficially I think we might be able to get around this by always
> doing the setup in the initial online. In brief that looks something the
> below code.  Relying on the cpu hotplug callback registration calling
> the acpi_soft_cpu_online for all instances that are already online.
>
> Very lightly tested on arm64 and x86 with cold and hotplugged CPUs.
> However this is all in emulation and I don't have access to any significant
> x86 test farms :( So help will be needed if it's not immediately obvious why
> we can't do this.

AFAICS, this should work.  At least I don't see why it wouldn't.

> Of course, I'm open to other suggestions!
>
> For now I'll put a tidied version of this one is as an RFC with the rest of v6.
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 06e718b650e5..97ca53b516d0 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -340,7 +340,7 @@ static int acpi_processor_get_info(struct acpi_device *device)
>          */
>         per_cpu(processor_device_array, pr->id) = device;
>         per_cpu(processors, pr->id) = pr;
> -
> +       pr->flags.need_hotplug_init = 1;
>         /*
>          *  Extra Processor objects may be enumerated on MP systems with
>          *  less than the max # of CPUs. They should be ignored _iff
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 67db60eda370..930f911fc435 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -206,7 +206,7 @@ static int acpi_processor_start(struct device *dev)
>
>         /* Protect against concurrent CPU hotplug operations */
>         cpu_hotplug_disable();
> -       ret = __acpi_processor_start(device);
> +       //      ret = __acpi_processor_start(device);
>         cpu_hotplug_enable();
>         return ret;
>  }

So it looks like acpi_processor_start() is not necessary any more, is it?

> @@ -279,7 +279,7 @@ static int __init acpi_processor_driver_init(void)
>         if (result < 0)
>                 return result;
>
> -       result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> +       result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>                                            "acpi/cpu-drv:online",
>                                            acpi_soft_cpu_online, NULL);
>         if (result < 0)
> >
> > Jonathan

Thanks!