Thread (16 messages) 16 messages, 5 authors, 2012-09-08

Re: [PATCH 1/2] PCI: Use local parameter pci_device_id for pci_get_subsys/class()

From: Yinghai Lu <yinghai@kernel.org>
Date: 2012-09-08 15:30:32
Also in: kvm, lkml

On Sat, Sep 8, 2012 at 6:42 AM, Fengguang Wu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Fri, Sep 07, 2012 at 06:32:48PM -0700, Yinghai Lu wrote:
quoted
quoted
with this one in pci/next pci config in /sys are not created.

10:~ # lspci -tv
pcilib: Cannot open /sys/bus/pci/devices/0000:00:03.0/config
lspci: Unable to read the standard configuration space header of
device 0000:00:03.0
pcilib: Cannot open /sys/bus/pci/devices/0000:00:02.0/config
lspci: Unable to read the standard configuration space header of
device 0000:00:02.0
pcilib: Cannot open /sys/bus/pci/devices/0000:00:01.3/config
lspci: Unable to read the standard configuration space header of
device 0000:00:01.3
pcilib: Cannot open /sys/bus/pci/devices/0000:00:01.1/config
lspci: Unable to read the standard configuration space header of
device 0000:00:01.1
pcilib: Cannot open /sys/bus/pci/devices/0000:00:01.0/config
lspci: Unable to read the standard configuration space header of
device 0000:00:01.0
pcilib: Cannot open /sys/bus/pci/devices/0000:00:00.0/config
lspci: Unable to read the standard configuration space header of
device 0000:00:00.0
-[0000:00]-

bisected to this commit

ccee7d23102f5e5765ec24779c5b77472af8f79e is the first bad commit
commit ccee7d23102f5e5765ec24779c5b77472af8f79e
Author: Feng Tang [off-list ref]
Date:   Thu Aug 23 15:45:03 2012 +0800

    PCI: Use pci_device_id on stack for pci_get_subsys/class() to avoid kmalloc

    This fixes a kernel warning https://lkml.org/lkml/2012/7/31/682

    pci_get_subsys() may get called in late system reboot stage, using
    a sleepable kmalloc() sounds fragile and will cause a kernel warning
    with my recent commmit 55c844a "x86/reboot: Fix a warning message
    triggered by stop_other_cpus()" which disable local interrupt in
    late system shutdown/reboot phase. Using a local parameter instead
    will fix it and make it eligible for calling forom atomic context.

    Do the same change for the pci_get_class() as suggested by Bjorn Helgaas

    [bhelgaas: changelog]
    Bisected-by: Fengguang Wu [off-list ref]
    Signed-off-by: Feng Tang [off-list ref]
    Signed-off-by: Bjorn Helgaas [off-list ref]
    Reviewed-by: Fengguang Wu [off-list ref]

:040000 040000 dee62a035816b73abc68e40de8f21c7349efc4cb
70b2a6258bffa1ab963bd650d8f5d02da774fbce M      drivers

so the stack get overrun ?

Bjorn, I think it is this one that cause lspci broken that I mentioned
during meeting at San Diego.
This makes lspci work again on my side. The caveat is, kzalloc() will
zero out all data while the new local variable leaves some data
uninitialized.
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 78a08b1..9148b6e 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -245,7 +245,12 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
                               unsigned int ss_vendor, unsigned int ss_device,
                               struct pci_dev *from)
 {
-       struct pci_device_id id;
+       struct pci_device_id id = {
+               .vendor = vendor,
+               .device = device,
+               .subvendor = ss_vendor,
+               .subdevice = ss_device,
+       };

        /*
         * pci_find_subsys() can be called on the ide_setup() path,
@@ -256,11 +261,6 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
        if (unlikely(no_pci_devices()))
                return NULL;

-       id.vendor = vendor;
-       id.device = device;
-       id.subvendor = ss_vendor;
-       id.subdevice = ss_device;
-
yes, here forget to clear .class and .class_mask
quoted hunk ↗ jump to hunk
        return pci_get_dev_by_id(&id, from);
 }
@@ -300,11 +300,14 @@ pci_get_device(unsigned int vendor, unsigned int device, struct pci_dev *from)
  */
 struct pci_dev *pci_get_class(unsigned int class, struct pci_dev *from)
 {
-       struct pci_device_id id;
-
-       id.vendor = id.device = id.subvendor = id.subdevice = PCI_ANY_ID;
-       id.class_mask = PCI_ANY_ID;
-       id.class = class;
+       struct pci_device_id id = {
+               .vendor = PCI_ANY_ID,
+               .device = PCI_ANY_ID,
+               .subvendor = PCI_ANY_ID,
+               .subdevice = PCI_ANY_ID,
+               .class_mask = PCI_ANY_ID,
+               .class = class,
+       };

        return pci_get_dev_by_id(&id, from);
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help