Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled
From: Bjorn Helgaas <bhelgaas@google.com>
Date: 2012-08-22 18:03:19
Also in:
kvm, lkml
On Wed, Aug 22, 2012 at 12:49 AM, Feng Tang [off-list ref] wrote:
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, FengguangThanks 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? 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().
quoted hunk ↗ jump to hunk
------------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) { struct pci_dev *pdev; - struct pci_device_id *id; + struct pci_device_id id; /* * pci_find_subsys() can be called on the ide_setup() path,@@ -257,17 +257,12 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, if (unlikely(no_pci_devices())) return NULL; - id = kzalloc(sizeof(*id), GFP_KERNEL); - if (!id) - return NULL; - id->vendor = vendor; - id->device = device; - id->subvendor = ss_vendor; - id->subdevice = ss_device; - - pdev = pci_get_dev_by_id(id, from); - kfree(id); + 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.
return pdev; }