Messages in this thread | | | Subject | Re: [PATCH] cpu/hotplug: Wait for cpu_hotplug to be enabled in cpu_up/down | From | Alexander Sverdlin <> | Date | Fri, 24 Jan 2020 12:51:09 +0100 |
| |
Hi!
On 24/01/2020 09:18, Matija Glavinic Pecotic wrote: > cpu hotplug may be disabled via cpu_hotplug_enable/cpu_hotplug_disable. > When disabled, cpu_down and cpu_up will fail with -EBUSY. Users of the > cpu_up/cpu_down should handle this situation as this is mostly temporal > disablement and exception should be made for EBUSY, assuming that EBUSY > always stands for this situation and is worth repeating execution. One > of the users of cpu_hotplug_enable/disable is pci_device_probe yielding > errors on bringing cpu cores up/down if pci devices are getting probed. > > Problem was observed on x86 board by having partitioning of the system > to RT/NRT cpu sets failing (of which part is to bring cpus down/up via > sysfs) if pci devices would be getting probed at the same time. This is > confusing for userspace as dependency to pci devices is not clear. > > Fix this behavior by waiting for cpu hotplug to be ready. Return -EBUSY > only after hotplugging was not enabled for about 10 seconds. > > Fixes: 1ddd45f8d76f ("PCI: Use cpu_hotplug_disable() instead of get_online_cpus()") > Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> --- > kernel/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 40 insertions(+), 10 deletions(-) > > diff --git a/kernel/cpu.c b/kernel/cpu.c > index 4dc279e..2e06ca9 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -31,6 +31,7 @@ > #include <linux/relay.h> > #include <linux/slab.h> > #include <linux/percpu-rwsem.h> > +#include <linux/wait.h> > > #include <trace/events/power.h> > #define CREATE_TRACE_POINTS > @@ -278,11 +279,22 @@ void cpu_maps_update_done(void) > } > > /* > - * If set, cpu_up and cpu_down will return -EBUSY and do nothing. > + * If set, cpu_up and cpu_down will retry for cpu_hotplug_retries and > + * eventually return -EBUSY if unsuccessful. > * Should always be manipulated under cpu_add_remove_lock > */ > static int cpu_hotplug_disabled; > > +/* > + * waitqueue for waiting on cpu_hotplug_disabled > + */ > +static DECLARE_WAIT_QUEUE_HEAD(wait_cpu_hp_enabled); > + > +/* > + * Retries for cpu_hotplug to be enabled by cpu_up/cpu_down. > + */ > +static int cpu_hotplug_retries = 10; > + > #ifdef CONFIG_HOTPLUG_CPU > > DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock); > @@ -341,7 +353,7 @@ static void lockdep_release_cpus_lock(void) > > /* > * Wait for currently running CPU hotplug operations to complete (if any) and > - * disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects > + * briefly disable CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects > * the 'cpu_hotplug_disabled' flag. The same lock is also acquired by the > * hotplug path before performing hotplug operations. So acquiring that lock > * guarantees mutual exclusion from any currently running hotplug operations. > @@ -366,6 +378,7 @@ void cpu_hotplug_enable(void) > cpu_maps_update_begin(); > __cpu_hotplug_enable(); > cpu_maps_update_done(); > + wake_up(&wait_cpu_hp_enabled); > } > EXPORT_SYMBOL_GPL(cpu_hotplug_enable); > > @@ -1044,11 +1057,21 @@ static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target) > > static int do_cpu_down(unsigned int cpu, enum cpuhp_state target) > { > - int err; > + int err = -EBUSY, retries = cpu_hotplug_retries; > > - cpu_maps_update_begin(); > - err = cpu_down_maps_locked(cpu, target); > - cpu_maps_update_done(); > + while (retries--) { > + wait_event_timeout(wait_cpu_hp_enabled, > + !cpu_hotplug_disabled, > + HZ); > + cpu_maps_update_begin(); > + if (cpu_hotplug_disabled) { > + cpu_maps_update_done(); > + continue; > + } > + err = _cpu_down(cpu, 0, target); > + cpu_maps_update_done(); > + break; > + } > return err; > } > > @@ -1166,7 +1189,7 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target) > > static int do_cpu_up(unsigned int cpu, enum cpuhp_state target) > { > - int err = 0; > + int err = 0, retries = cpu_hotplug_retries; > > if (!cpu_possible(cpu)) { > pr_err("can't online cpu %d because it is not configured as may-hotadd at boot time\n", > @@ -1181,9 +1204,16 @@ static int do_cpu_up(unsigned int cpu, enum cpuhp_state target) > if (err) > return err; > > - cpu_maps_update_begin(); > - > - if (cpu_hotplug_disabled) { > + while (--retries) { > + wait_event_timeout(wait_cpu_hp_enabled, > + !cpu_hotplug_disabled, > + HZ); > + cpu_maps_update_begin(); > + if (!cpu_hotplug_disabled) > + break; > + cpu_maps_update_done(); > + } > + if (!retries) { > err = -EBUSY; > goto out; > } >
-- Best regards, Alexander Sverdlin.
| |