On Thursday, October 23, 2014 12:11:18 PM Bjorn Helgaas wrote:
> On Wed, Oct 22, 2014 at 03:34:56PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, October 22, 2014 02:31:55 PM Lucas Stach wrote:
> > > If the irqchip handling the PCIe PME interrupt is not able
> > > to enable interrupt wakeup we should properly reflect this
> > > in the PME suspend status.
> > >
> > > This fixes a kernel warning on resume, where it would try
> > > to disable the irq wakeup that failed to be activated while
> > > suspending. The issue was introduced with 76cde7e49590
> > > (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle).
> > >
> > > Reported-by: Richard Zhu <richard.zhu@xxxxxxxxxxxxx>
> > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > > Tested-by: Richard Zhu <richard.zhu@xxxxxxxxxxxxx>
> >
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Bjorn, do you want me to handle this or will you take it?
>
> It fixes 76cde7e49590, which it looks like you merged for v3.18-rc1, so
> probably this should be merged as a fix before v3.18, right? If you agree,
> you can go ahead and merge it since you merged the original.
OK, I've queued it up for my next pull request (which most likely will be sent
tomorrow).
> It would be nice to have the actual warning, e.g., just the "Unbalanced IRQ
> 384 wake disable" part, in the changelog to help correlate this with the
> place where the problem is detected.
Done.
Rafael
> > > ---
> > > Trimmed warning on resume looks like this:
> > > [ 109.292736] WARNING: CPU: 0 PID: 609 at kernel/irq/manage.c:536 irq_set_irq_wake+0xc0/0xf8()
> > > [ 109.301193] Unbalanced IRQ 384 wake disable
> > > [ 109.305392] Modules linked in:
> > > [ 109.308502] CPU: 0 PID: 609 Comm: kworker/u2:9 Tainted: G W 3.18.0-rc1-00009-g820df3d-dirty #268
> > > [ 109.318368] Workqueue: events_unbound async_run_entry_fn
> > > [ 109.323718] Backtrace:
> > > [ 109.326233] [<80012460>] (dump_backtrace) from [<80012744>] (show_stack+0x18/0x1c)
> > > [ 109.339616] [<8001272c>] (show_stack) from [<806d8dc8>] (dump_stack+0x8c/0xa4)
> > > [ 109.346885] [<806d8d3c>] (dump_stack) from [<8002a88c>] (warn_slowpath_common+0x70/0x94)
> > > [ 109.360773] [<8002a81c>] (warn_slowpath_common) from [<8002a8e8>] (warn_slowpath_fmt+0x38/0x40)
> > > [ 109.376334] [<8002a8b4>] (warn_slowpath_fmt) from [<8006c2a8>] (irq_set_irq_wake+0xc0/0xf8)
> > > [ 109.388351] [<8006c1e8>] (irq_set_irq_wake) from [<802f22cc>] (pcie_pme_resume+0x34/0x64)
> > > [ 109.402328] [<802f2298>] (pcie_pme_resume) from [<802f1590>] (resume_iter+0x44/0x50)
> > > [ 109.413742] [<802f154c>] (resume_iter) from [<803784d4>] (device_for_each_child+0x4c/0x78)
> > > [ 109.422039] [<80378488>] (device_for_each_child) from [<802f196c>] (pcie_port_device_resume+0x18/0x20)
> > > [ 109.436085] [<802f1954>] (pcie_port_device_resume) from [<802e6f40>] (pci_pm_resume+0x7c/0x10c)
> > > [ 109.444829] [<802e6ec4>] (pci_pm_resume) from [<8038502c>] (dpm_run_callback.isra.12+0x34/0x7c)
> > > [ 109.459323] [<80384ff8>] (dpm_run_callback.isra.12) from [<8038543c>] (device_resume+0xb8/0x194)
> > > [ 109.474961] [<80385384>] (device_resume) from [<80385538>] (async_resume+0x20/0x4c)
> > > [ 109.490523] [<80385518>] (async_resume) from [<800471a0>] (async_run_entry_fn+0x48/0x11c)
> > > [ 109.502371] [<80047158>] (async_run_entry_fn) from [<8003f46c>] (process_one_work+0x1ac/0x414)
> > > [ 109.515711] [<8003f2c0>] (process_one_work) from [<8003ff50>] (worker_thread+0x148/0x4e0)
> > > [ 109.534446] [<8003fe08>] (worker_thread) from [<80044704>] (kthread+0xdc/0xf8)
> > > [ 109.552225] [<80044628>] (kthread) from [<8000ec08>] (ret_from_fork+0x14/0x2c)
> > > ---
> > > drivers/pci/pcie/pme.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
> > > index a9f9c46e5022..63fc63911295 100644
> > > --- a/drivers/pci/pcie/pme.c
> > > +++ b/drivers/pci/pcie/pme.c
> > > @@ -397,6 +397,7 @@ static int pcie_pme_suspend(struct pcie_device *srv)
> > > struct pcie_pme_service_data *data = get_service_data(srv);
> > > struct pci_dev *port = srv->port;
> > > bool wakeup;
> > > + int ret;
> > >
> > > if (device_may_wakeup(&port->dev)) {
> > > wakeup = true;
> > > @@ -407,9 +408,10 @@ static int pcie_pme_suspend(struct pcie_device *srv)
> > > }
> > > spin_lock_irq(&data->lock);
> > > if (wakeup) {
> > > - enable_irq_wake(srv->irq);
> > > + ret = enable_irq_wake(srv->irq);
> > > data->suspend_level = PME_SUSPEND_WAKEUP;
> > > - } else {
> > > + }
> > > + if (!wakeup || ret) {
> > > struct pci_dev *port = srv->port;
> > >
> > > pcie_pme_interrupt_enable(port, false);
> > >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
