[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1843565.7Y1sC8j6FG@vostro.rjw.lan>
Date: Mon, 11 Feb 2013 20:49:49 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Bjorn Helgaas <bhelgaas@...gle.com>,
Daniel J Blueman <daniel@...ra.org>
Cc: Linux Kernel <linux-kernel@...r.kernel.org>,
Linux PCI <linux-pci@...r.kernel.org>,
Yijing Wang <wangyijing@...wei.com>
Subject: Re: [3.8-rc7] PCI hotplug wakeup oops
On Monday, February 11, 2013 08:27:49 PM Rafael J. Wysocki wrote:
> On Monday, February 11, 2013 12:01:37 PM Bjorn Helgaas wrote:
> > [+cc Rafael]
> >
> > On Mon, Feb 11, 2013 at 10:08 AM, Daniel J Blueman <daniel@...ra.org> wrote:
> > > On 11 February 2013 21:03, Daniel J Blueman <daniel@...ra.org> wrote:
> > >> With 3.8-rc7, when unplugging the Thunderbolt ethernet adapter (bus 0a
> > >> [1]) on a Macbook Pro 10,1, we see the PCIe port correctly released:
> > >>
> > >> pciehp 0000:06:03.0:pcie24: Card not present on Slot(3)
> > >> tg3 0000:0a:00.0: tg3_abort_hw timed out, TX_MODE_ENABLE will not
> > >> clear MAC_TX_MODE=ffffffff
> > >> tg3 0000:0a:00.0 eth0: No firmware running
> > >> tg3 0000:0a:00.0 eth0: Link is down
> > >> [sched_delayed] sched: RT throttling activated
> > >> pcieport 0000:00:01.1: System wakeup enabled by ACPI
> > >> pciehp 0000:09:00.0:pcie24: unloading service driver pciehp
> > >> pci_bus 0000:0a: busn_res: [bus 0a] is released
> > >> pci_bus 0000:09: busn_res: [bus 09-0a] is released
> > >>
> > >> After some activity later (eg I can reproduce this by switching to a
> > >> text console and back), often we'll see an oops:
> > >>
> > >> Unable to handle kernel paging request at 0000000000001070
> > >> pci_pme_list_scan+0x3d/0xe0
> > >> Call Trace:
> > >> process_one_work+0x193
> > >> ? process_one_work+0x131
> > >> ? pci_pme_wakeup+0x60
> > >> worker_thread+0x15d
> > >>
> > >> (gdb) list *(pci_pme_list_scan+0x3d)
> > >> 0xffffffff8123f6dd is in pci_pme_list_scan (drivers/pci/pci.c:1556).
> > >> 1551 /*
> > >> 1552 * If bridge is in low power state, the
> > >> 1553 * configuration space of subordinate devices
> > >> 1554 * may be not accessible
> > >> 1555 */
> > >> 1556 if (bridge && bridge->current_state != PCI_D0)
> > >> 1557 continue;
> > >> 1558 pci_pme_wakeup(pme_dev->dev, NULL);
> > >> 1559 } else {
> > >> 1560 list_del(&pme_dev->list);
> > >>
> > >> Since a panic in vsnprintf happens after the oops (hence I can't catch
> > >> it with EFI pstore), it is almost certainly significant heap
> > >> corruption; this would explain why pme_dev became null (the load has
> > >> been ordered ahead).
> > >>
> > >> I'll see what I can find out with memory poisoning and list debugging.
> > >
> > > Enabling a bunch of related debugging, we see pme_dev is non-null and:
> > >
> > > BUG: Unable to handle NULL pointer dereference at
> > > pci_bus_read_config_word+0x6c
> > > PGD 26314c067 PUD 2633f9067 PMD 0
> > > Oops: 0000 [#1] SMP
> > > pci_check_pme_status+0x4f
> > > pci_pme_wakeup+0x21
> > > pci_pme_list_scan+0xd5
> > > process_one_work+0x1ca
> > > ? process_one_work+0x160
> > > ? pci_pme_wakeup+0x60
> > > worker_thread+0x14e
> > >
> > > Anyway, it looks like the device being unplugged wasn't removed from
> > > pci_pme_list as pci_pme_active(dev, false) wasn't called.
> > >
> > > From a quick review, I wasn't able to find the right place in the
> > > call-chain which I only see releases the child busses and PCIe port
> > > drivers. Anyone?
> >
> > It looks like drivers *add* devices to pci_pme_list when they use
> > pci_enable_wake() or pci_wake_from_d3(). But many drivers never
> > remove their devices, and I don't see any place where the core does it
> > either. My guess is we need to remove it in pci_stop_dev() (we
> > already do pcie_aspm_exit_link_state() there) or somewhere similar.
>
> Yes, we should call pci_pme_active(dev, false) somewhere in there I think.
> It's fine to call that even if PME was not "active" before.
Daniel, I wonder if the patch below helps?
Rafael
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
drivers/pci/remove.c | 2 ++
1 file changed, 2 insertions(+)
Index: test/drivers/pci/remove.c
===================================================================
--- test.orig/drivers/pci/remove.c
+++ test/drivers/pci/remove.c
@@ -19,6 +19,8 @@ static void pci_free_resources(struct pc
static void pci_stop_dev(struct pci_dev *dev)
{
+ pci_pme_active(dev, false);
+
if (dev->is_added) {
pci_proc_detach_device(dev);
pci_remove_sysfs_dev_files(dev);
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists