Thread (29 messages) 29 messages, 8 authors, 2012-09-08

Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

From: Feng Tang <hidden>
Date: 2012-08-23 05:51:32
Also in: linux-pci, lkml

Hi Bjorn,

On Wed, 22 Aug 2012 11:02:52 -0700
Bjorn Helgaas [off-list ref] wrote:
On Wed, Aug 22, 2012 at 12:49 AM, Feng Tang [off-list ref] wrote:
quoted
Hi Fengguang,


On Wed, 22 Aug 2012 10:50:08 +0800
Fengguang Wu [off-list ref] wrote:
quoted
Feng,
quoted
I think it's pci_get_subsys() triggered this assert:

        /*
         * Oi! Can't be having __GFP_FS allocations with IRQs disabled.
         */
        if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)))
                return;
It's bisected down to this commit:

commit 55c844a4dd16a4d1fdc0cf2a283ec631a02ec448
Author:     Feng Tang [off-list ref]
AuthorDate: Wed May 30 23:15:41 2012 +0800
Commit:     Ingo Molnar [off-list ref]
CommitDate: Wed Jun 6 12:03:23 2012 +0200

    x86/reboot: Fix a warning message triggered by stop_other_cpus()

Thanks,
Fengguang
Thanks for the bisection.

Revert my commit should be a solution, but can we simply make the pci_device_id
a local on stack one instead of using sleepable kmalloc for it, as this
sounds fragile when pci_get_subsys get called in a late system reboot stage?
I think this is a great idea.  Can you make this a real patch, with a
changelog and Signed-off-by?
Thanks and will do.
We should also remove the obsolete comment about early boot.  I'm not
sure the no_pci_devices() check is needed, either.  And we can make
the same simplification in pci_get_class().
Will check the no_pci_devices() part, and try to make it a separate
patch for easy reverting in case of error.
quoted
------------
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 993d4a0..e5ccede 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -246,7 +246,7 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
                               struct pci_dev *from)
 {
+       id.vendor = vendor;
+       id.device = device;
+       id.subvendor = ss_vendor;
+       id.subdevice = ss_device;

+       pdev = pci_get_dev_by_id(&id, from);
No need for "pdev" here, since we don't have to free anything.
ok, will directly return it.

Thanks,
Feng
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help