Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1625577
| From | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() |
| Date | 2017-04-18 21:50 +0200 |
| Message-ID | <txHiq-1Gh-17@gated-at.bofh.it> (permalink) |
| References | <twzPX-123-3@gated-at.bofh.it> <twzPY-123-13@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Sat, Apr 15, 2017 at 07:01:24PM +0200, Thomas Gleixner wrote:
> Converting the hotplug locking, i.e. get_online_cpus(), to a percpu rwsem
> unearthed a circular lock dependency which was hidden from lockdep due to
> the lockdep annotation of get_online_cpus() which prevents lockdep from
> creating full dependency chains. There are several variants of this. And
> example is:
>
> Chain exists of:
>
> cpu_hotplug_lock.rw_sem --> drm_global_mutex --> &item->mutex
>
> CPU0 CPU1
> ---- ----
> lock(&item->mutex);
> lock(drm_global_mutex);
> lock(&item->mutex);
> lock(cpu_hotplug_lock.rw_sem);
>
> because there are dependencies through workqueues. The call chain is:
>
> get_online_cpus
> apply_workqueue_attrs
> __alloc_workqueue_key
> ttm_mem_global_init
> ast_ttm_mem_global_init
> drm_global_item_ref
> ast_mm_init
> ast_driver_load
> drm_dev_register
> drm_get_pci_dev
> ast_pci_probe
> local_pci_probe
> work_for_cpu_fn
> process_one_work
> worker_thread
>
> This is not a problem of get_online_cpus() recursion, it's a possible
> deadlock undetected by lockdep so far.
>
> The cure is to use cpu_hotplug_disable() instead of get_online_cpus() to
> protect the PCI probing.
>
> There is a side effect to this: cpu_hotplug_disable() makes a concurrent
> cpu hotplug attempt via the sysfs interfaces fail with -EBUSY, but PCI
> probing usually happens during the boot process where no interaction is
> possible. Any later invocations are infrequent enough and concurrent
> hotplug attempts are so unlikely that the danger of user space visible
> regressions is very close to zero. Anyway, thats preferrable over a real
> deadlock.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
> drivers/pci/pci-driver.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -320,10 +320,19 @@ static long local_pci_probe(void *_ddi)
> return 0;
> }
>
> +static bool pci_physfn_is_probed(struct pci_dev *dev)
> +{
> +#ifdef CONFIG_ATS
I think this was intended to be CONFIG_PCI_ATS, not CONFIG_ATS.
But I think CONFIG_PCI_IOV would be more appropriate. With that, and
squashing this into the next patch,
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
I expect you'll merge this along with the rest of the series. Let me
know if you need anything else from me.
> + return dev->physfn->is_probed;
> +#else
> + return false;
> +#endif
> +}
> +
> static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
> const struct pci_device_id *id)
> {
> - int error, node;
> + int error, node, cpu;
> struct drv_dev_and_id ddi = { drv, dev, id };
>
> /*
> @@ -349,13 +358,13 @@ static int pci_call_probe(struct pci_dri
> if (node >= 0 && node != numa_node_id()) {
> int cpu;
>
> - get_online_cpus();
> + cpu_hotplug_disable();
> cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
> if (cpu < nr_cpu_ids)
> error = work_on_cpu(cpu, local_pci_probe, &ddi);
> else
> error = local_pci_probe(&ddi);
> - put_online_cpus();
> + cpu_hotplug_enable();
> } else
> error = local_pci_probe(&ddi);
>
>
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[patch 00/20] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 02/20] stop_machine: Provide stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 20/20] cpu/hotplug: Convert hotplug locking to percpu rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
Re: [patch 20/20] cpu/hotplug: Convert hotplug locking to percpu rwsem Peter Zijlstra <peterz@infradead.org> - 2017-04-17 09:00 +0200
[patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() Peter Zijlstra <peterz@infradead.org> - 2017-04-17 08:50 +0200
Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-17 09:50 +0200
Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() Bjorn Helgaas <helgaas@kernel.org> - 2017-04-18 21:50 +0200
Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
[patch 12/20] s390/kernel: Use stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 13/20] powerpc/powernv: Use stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 03/20] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
Re: [patch 03/20] padata: Make padata_alloc() static "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-04-16 08:30 +0200
Re: [patch 03/20] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-04-17 11:20 +0200
[patch 01/20] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 18/20] PCI: Replace the racy recursion prevention Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 04/20] padata: Avoid nested calls to get_online_cpus() in pcrypt_init_padata() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 14/20] kernel/hotplug: Use stop_machine_locked() in takedown_cpu() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 05/20] x86/mtrr: Remove get_online_cpus() from mtrr_save_state() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 07/20] KVM/PPC/Book3S HV: Use cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 16/20] perf/x86/intel: Drop get_online_cpus() in intel_snb_check_microcode() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
Re: [patch 16/20] perf/x86/intel: Drop get_online_cpus() in intel_snb_check_microcode() Borislav Petkov <bp@alien8.de> - 2017-04-18 13:30 +0200
[patch 09/20] hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 10/20] perf/x86/intel/cqm: Use cpuhp_setup_state_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
Re: [patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-04-16 01:00 +0200
Re: [patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-17 06:20 +0200
[patch 08/20] hwtracing/coresight-etm3x: Use the locked version of cpuhp_setup_state_nocalls() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
[patch 11/20] ARM/hw_breakpoint: Use cpuhp_setup_state_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
csiph-web