Revert commit ef83b0781a "PCI: Remove from bus_list and release
resources in pci_release_dev()" that made some nasty race conditions
become possible. For example, if a Thunderbolt link is unplugged
and then replugged immediately, the pci_release_dev() resulting from
the hot-remove code path may be racing with the hot-add code path
which after that commit causes various kinds of breakage to happen
(up to and including a hard crash of the whole system).
Moreover, the problem that commit ef83b0781a attempted to address
cannot happen any more after commit 8a4c5c329d "PCI: Check parent
kobject in pci_destroy_dev()", because pci_destroy_dev() will now
return immediately if it has already been executed for the given
device.
Note, however, that the invocation of msi_remove_pci_irq_vectors()
removed by commit ef83b0781a from pci_free_resources() along with
the other changes made by it is not added back because of subsequent
code changes depending on that modification.
Fixes: ef83b0781a (PCI: Remove from bus_list and release resources in pci_release_dev())
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
If pci_stop_and_remove_bus_device() is run concurrently for a device and
its parent bridge via remove_callback(), both code paths attempt to acquire
pci_rescan_remove_lock. If the child device removal acquires it first,
there will be no problems. However, if the parent bridge removal acquires
it first, it will eventually execute pci_destroy_dev() for the child
device, but that device object will not be freed yet due to the reference
held by the concurrent child removal. Consequently, both
pci_stop_bus_device() and pci_remove_bus_device() will be executed for that
device unnecessarily and pci_destroy_dev() will see a corrupted list head
in that object. Moreover, an excess put_device() will be executed for that
device in that case which may lead to a use-after-free in the final
kobject_put() done by sysfs_schedule_callback_work().
To avoid that problem, make pci_destroy_dev() check if the device's parent
kobject is NULL, which only happens after device_del() has already run for
it. Make pci_destroy_dev() return immediately whithout doing anything in
that case.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
There are multiple PCI device addition and removal code paths that may be
run concurrently with the generic PCI bus rescan and device removal that
can be triggered via sysfs. If that happens, it may lead to multiple
different, potentially dangerous race conditions.
The most straightforward way to address those problems is to run
the code in question under the same lock that is used by the
generic rescan/remove code in pci-sysfs.c. To prepare for those
changes, move the definition of the global PCI remove/rescan lock
to probe.c and provide global wrappers, pci_lock_rescan_remove()
and pci_unlock_rescan_remove(), allowing drivers to manipulate
that lock. Also provide pci_stop_and_remove_bus_device_locked()
for the callers of pci_stop_and_remove_bus_device() who only need
to hold the rescan/remove lock around it.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Previously we removed the pci_dev from the bus_list and released its
resources in pci_destroy_dev(). But that's too early: it's possible to
call pci_destroy_dev() twice for the same device (e.g., via sysfs), and
that will cause an oops when we try to remove it from bus_list the second
time.
We should remove it from the bus_list only when the last reference to the
pci_dev has been released, i.e., in pci_release_dev().
[bhelgaas: changelog]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
To be consistent with 4bff674990 ("PCI: Move device_del() from
pci_stop_dev() to pci_destroy_dev()", this changes pci_stop_root_bus()
to use device_release_driver() instead of device_del().
This also changes pci_remove_root_bus() to use device_unregister()
instead of put_device() so it corresponds with the device_register()
call in pci_create_root_bus().
[bhelgaas: changelog]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
After commit bcdde7e221 (sysfs: make __sysfs_remove_dir() recursive)
I'm seeing traces analogous to the one below in Thunderbolt testing:
WARNING: CPU: 3 PID: 76 at /scratch/rafael/work/linux-pm/fs/sysfs/group.c:214 sysfs_remove_group+0x59/0xe0()
sysfs group ffffffff81c6c500 not found for kobject '0000:08'
Modules linked in: ...
CPU: 3 PID: 76 Comm: kworker/u16:7 Not tainted 3.13.0-rc1+ #76
Hardware name: Acer Aspire S5-391/Venus , BIOS V1.02 05/29/2012
Workqueue: kacpi_hotplug acpi_hotplug_work_fn
0000000000000009 ffff8801644b9ac8 ffffffff816b23bf 0000000000000007
ffff8801644b9b18 ffff8801644b9b08 ffffffff81046607 ffff88016925b800
0000000000000000 ffffffff81c6c500 ffff88016924f928 ffff88016924f800
Call Trace:
[<ffffffff816b23bf>] dump_stack+0x4e/0x71
[<ffffffff81046607>] warn_slowpath_common+0x87/0xb0
[<ffffffff810466d1>] warn_slowpath_fmt+0x41/0x50
[<ffffffff811e42ef>] ? sysfs_get_dirent_ns+0x6f/0x80
[<ffffffff811e5389>] sysfs_remove_group+0x59/0xe0
[<ffffffff8149f00b>] dpm_sysfs_remove+0x3b/0x50
[<ffffffff81495818>] device_del+0x58/0x1c0
[<ffffffff814959c8>] device_unregister+0x48/0x60
[<ffffffff813254fe>] pci_remove_bus+0x6e/0x80
[<ffffffff81325548>] pci_remove_bus_device+0x38/0x110
[<ffffffff8132555d>] pci_remove_bus_device+0x4d/0x110
[<ffffffff81325639>] pci_stop_and_remove_bus_device+0x19/0x20
[<ffffffff813418d0>] disable_slot+0x20/0xe0
[<ffffffff81341a38>] acpiphp_check_bridge+0xa8/0xd0
[<ffffffff813427ad>] hotplug_event+0x17d/0x220
[<ffffffff81342880>] hotplug_event_work+0x30/0x70
[<ffffffff8136d665>] acpi_hotplug_work_fn+0x18/0x24
[<ffffffff81061331>] process_one_work+0x261/0x450
[<ffffffff81061a7e>] worker_thread+0x21e/0x370
[<ffffffff81061860>] ? rescuer_thread+0x300/0x300
[<ffffffff81068342>] kthread+0xd2/0xe0
[<ffffffff81068270>] ? flush_kthread_worker+0x70/0x70
[<ffffffff816c19bc>] ret_from_fork+0x7c/0xb0
[<ffffffff81068270>] ? flush_kthread_worker+0x70/0x70
(Mika Westerberg sees them too in his tests).
Some investigation documented in kernel bug #65281 led me to the
conclusion that the source of the problem is the device_del() in
pci_stop_dev() as it now causes the sysfs directory of the device to be
removed recursively along with all of its subdirectories. That includes
the sysfs directory of the device's subordinate bus (dev->subordinate) and
its "power" group.
Consequently, when pci_remove_bus() is called for dev->subordinate in
pci_remove_bus_device(), it calls device_unregister(&bus->dev), but at this
point the sysfs directory of bus->dev doesn't exist any more and its
"power" group doesn't exist either. Thus, when dpm_sysfs_remove() called
from device_del() tries to remove that group, it triggers the above
warning.
That indicates a logical mistake in the design of
pci_stop_and_remove_bus_device(), which causes bus device objects to be
left behind their parents (bridge device objects) and can be fixed by
moving the device_del() from pci_stop_dev() into pci_destroy_dev(), so
pci_remove_bus() can be called for the device's subordinate bus before the
device itself is unregistered from the hierarchy. Still, the driver, if
any, should be detached from the device in pci_stop_dev(), so use
device_release_driver() directly from there.
References: https://bugzilla.kernel.org/show_bug.cgi?id=65281#c6
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Fix whitespace, capitalization, and spelling errors. No functional change.
I know "busses" is not an error, but "buses" was more common, so I used it
consistently.
Signed-off-by: Marta Rybczynska <rybczynska@gmail.com> (pci_reset_bridge_secondary_bus())
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
On ACPI-based platforms, the pci_slot driver creates PCI slot devices
according to information from ACPI tables by registering an ACPI PCI
subdriver. The ACPI PCI subdriver will only be called when creating/
destroying PCI root buses, and it won't be called when hot-plugging
P2P bridges. It may cause stale PCI slot devices after hot-removing
a P2P bridge if that bridge has associated PCI slots. And the acpiphp
driver has the same issue too.
This patch introduces two hook points into the PCI core, which will
be invoked when creating/destroying PCI buses for PCI host and P2P
bridges. They could be used to setup/destroy platform dependent stuff
in a unified way, both at boot time and for PCI hotplug operations.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Yinghai Lu <yinghai@kernel.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Myron Stowe <myron.stowe@redhat.com>
We always call device_register() and pci_create_legacy_files() for a
new bus before handing out the "struct pci_bus *". Therefore, there's
no possiblity of removing the bus with pci_remove_bus() before those
calls have been made, so we don't need to check "bus->is_added" before
calling pci_remove_legacy_files() and device_unregister().
[bhelgaas: changelog]
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Yinghai Lu <yinghai@kernel.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Pull PCI changes from Bjorn Helgaas:
"Host bridge hotplug
- Major overhaul of ACPI host bridge add/start (Rafael Wysocki, Yinghai Lu)
- Major overhaul of PCI/ACPI binding (Rafael Wysocki, Yinghai Lu)
- Split out ACPI host bridge and ACPI PCI device hotplug (Yinghai Lu)
- Stop caching _PRT and make independent of bus numbers (Yinghai Lu)
PCI device hotplug
- Clean up cpqphp dead code (Sasha Levin)
- Disable ARI unless device and upstream bridge support it (Yijing Wang)
- Initialize all hot-added devices (not functions 0-7) (Yijing Wang)
Power management
- Don't touch ASPM if disabled (Joe Lawrence)
- Fix ASPM link state management (Myron Stowe)
Miscellaneous
- Fix PCI_EXP_FLAGS accessor (Alex Williamson)
- Disable Bus Master in pci_device_shutdown (Konstantin Khlebnikov)
- Document hotplug resource and MPS parameters (Yijing Wang)
- Add accessor for PCIe capabilities (Myron Stowe)
- Drop pciehp suspend/resume messages (Paul Bolle)
- Make pci_slot built-in only (not a module) (Jiang Liu)
- Remove unused PCI/ACPI bind ops (Jiang Liu)
- Removed used pci_root_bus (Bjorn Helgaas)"
* tag 'pci-v3.9-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci: (51 commits)
PCI/ACPI: Don't cache _PRT, and don't associate them with bus numbers
PCI: Fix PCI Express Capability accessors for PCI_EXP_FLAGS
ACPI / PCI: Make pci_slot built-in only, not a module
PCI/PM: Clear state_saved during suspend
PCI: Use atomic_inc_return() rather than atomic_add_return()
PCI: Catch attempts to disable already-disabled devices
PCI: Disable Bus Master unconditionally in pci_device_shutdown()
PCI: acpiphp: Remove dead code for PCI host bridge hotplug
PCI: acpiphp: Create companion ACPI devices before creating PCI devices
PCI: Remove unused "rc" in virtfn_add_bus()
PCI: pciehp: Drop suspend/resume ENTRY messages
PCI/ASPM: Don't touch ASPM if forcibly disabled
PCI/ASPM: Deallocate upstream link state even if device is not PCIe
PCI: Document MPS parameters pci=pcie_bus_safe, pci=pcie_bus_perf, etc
PCI: Document hpiosize= and hpmemsize= resource reservation parameters
PCI: Use PCI Express Capability accessor
PCI: Introduce accessor to retrieve PCIe Capabilities Register
PCI: Put pci_dev in device tree as early as possible
PCI: Skip attaching driver in device_add()
PCI: acpiphp: Keep driver loaded even if no slots found
...
Devices are added to pci_pme_list when drivers use pci_enable_wake()
or pci_wake_from_d3(), but they aren't removed from the list unless
the driver explicitly disables wakeup. Many drivers never disable
wakeup, so their devices remain on the list even after they are
removed, e.g., via hotplug. A subsequent PME poll will oops when
it tries to touch the device.
This patch disables PME# on a device before removing it, which removes
the device from pci_pme_list. This is safe even if the device never
had PME# enabled.
This oops can be triggered by unplugging a Thunderbolt ethernet adapter
on a Macbook Pro, as reported by Daniel below.
[bhelgaas: changelog]
Reference: http://lkml.kernel.org/r/CAMVG2svG21yiM1wkH4_2pen2n+cr2-Zv7TbH3Gj+8MwevZjDbw@mail.gmail.com
Reported-and-tested-by: Daniel J Blueman <daniel@quora.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: stable@vger.kernel.org
According to device model documentation, the way to create/destroy PCI
devices should be symmetric. The rule is to either use
1) device_register()/device_unregister()
or
2) device_initialize()/device_add()/device_del()/put_device().
So change PCI core logic to follow the rule and get rid of the redundant
pci_dev_get()/pci_dev_put() pair.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It supports both PCI root bus and PCI bus under PCI bridge.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
This restores the previous behavior of stopping all child devices before
removing any of them. The current SR-IOV design, where removing the PF
also drops references on all the VFs, depends on having the VFs continue
to exist after having been stopped.
[bhelgaas: changelog]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
list_del() already sets next/prev to LIST_POISON1/LIST_POISON2, so we
don't need to do anything special here to prevent further list accesses.
Tested-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
Previously, when we removed a PCI device, we made two passes over the
hierarchy rooted at the device. In the first pass, we stopped all
the devices, and in the second, we removed them.
This patch combines the two passes into one so that we remove a device as
soon as it and all its children have been stopped.
Note that we previously stopped devices in reverse order and removed them
in forward order. Now we stop and remove them in reverse order.
Tested-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
pci_stop_bus_devices() is only two lines of code and is only called by
pci_stop_bus_device(), so I think it's easier to read if we just fold it
into the caller. Similarly for __pci_remove_behind_bridge().
Tested-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
The PCMCIA CardBus driver was the only user of
pci_stop_and_remove_behind_bridge(), and it now uses
pci_stop_and_remove_bus_device() instead, so remove this interface.
This removes exported symbol pci_stop_and_remove_behind_bridge.
Tested-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
The acpiphp hotplug driver was the only user of pci_stop_bus_device() and
__pci_remove_bus_device(), and it now uses pci_stop_and_remove_bus_device()
instead, so stop exposing these interfaces.
This removes these exported symbols:
__pci_remove_bus_device
pci_stop_bus_device
Tested-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
Don't switch to pci_remove_bus_device yet, keep the __ prefix for now
(the behavior is still the same: remove without stopping first).
This allows other out of tree users or pending patches to get notified
from compiler warning.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
The old pci_remove_behind_bridge actually do stop and remove.
Make the name reflect that to reduce confusion.
Suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Rafael J. Wysocki
Yinghai Lu
Bjorn Helgaas
Jiang Liu
Linus Torvalds
Rafael J. Wysocki
Jiang Liu