On Tue, Apr 16, 2024 at 7:41 PM Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote: > > On Mon, 15 Apr 2024 13:23:51 +0100 > Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> wrote: > > > On Mon, 15 Apr 2024 14:04:26 +0200 > > "Rafael J. Wysocki" <rafael@xxxxxxxxxx> 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!
