2012-08-22 02:50:37

by Fengguang Wu

[permalink] [raw]
Subject: Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

Feng,

> 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 <[email protected]>
AuthorDate: Wed May 30 23:15:41 2012 +0800
Commit: Ingo Molnar <[email protected]>
CommitDate: Wed Jun 6 12:03:23 2012 +0200

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

Thanks,
Fengguang

> [ 91.282131] machine restart
> [ 91.283895] ------------[ cut here ]------------
> [ 91.284731] WARNING: at /c/wfg/linux/kernel/lockdep.c:2739 lockdep_trace_alloc+0x1fb/0x210()
> [ 91.286132] Modules linked in:
> [ 91.286703] Pid: 697, comm: reboot Not tainted 3.5.0-00024-g01ff5db-dirty #4
> [ 91.287859] Call Trace:
> [ 91.288289] [<81050148>] warn_slowpath_common+0xb8/0x100
> [ 91.289338] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> [ 91.290264] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> [ 91.291161] [<810501ce>] warn_slowpath_null+0x3e/0x50
> [ 91.292042] [<8110acdb>] lockdep_trace_alloc+0x1fb/0x210
> [ 91.292934] [<81228e25>] kmem_cache_alloc_trace+0x55/0x600
> [ 91.292934] [<813025ca>] ? kobject_put+0x9a/0x160
> [ 91.292934] [<814e95e0>] ? klist_iter_exit+0x30/0x50
> [ 91.292934] [<81405881>] ? bus_find_device+0xf1/0x120
> [ 91.292934] [<81361a3c>] ? pci_get_subsys+0x11c/0x1b0
> [ 91.292934] [<81361a3c>] pci_get_subsys+0x11c/0x1b0
> [ 91.292934] [<81361afe>] pci_get_device+0x2e/0x40
> [ 91.292934] [<81033e25>] mach_reboot_fixups+0xa5/0xd0
> [ 91.292934] [<81027611>] native_machine_emergency_restart+0x1f1/0x590
> [ 91.292934] [<814f2e00>] ? printk+0x4b/0x5b
> [ 91.292934] [<810269ef>] native_machine_restart+0x6f/0x80
> [ 91.292934] [<810271cc>] machine_restart+0x1c/0x30
> [ 91.292934] [<810886e0>] kernel_restart+0x70/0xc0
> [ 91.292934] [<81088a85>] sys_reboot+0x325/0x380
> [ 91.292934] [<811f796c>] ? handle_pte_fault+0xdc/0x1740
> [ 91.292934] [<811f93e7>] ? handle_mm_fault+0x417/0x4a0
> [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> [ 91.292934] [<810b33e7>] ? up_read+0x37/0x70
> [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> [ 91.292934] [<810b0270>] ? update_rmtp+0xe0/0xe0
> [ 91.292934] [<8150376e>] ? restore_all+0xf/0xf
> [ 91.292934] [<8103d880>] ? vmalloc_sync_all+0x320/0x320
> [ 91.292934] [<81109fca>] ? trace_hardirqs_on_caller+0x28a/0x380
> [ 91.292934] [<81311594>] ? trace_hardirqs_on_thunk+0xc/0x10
> [ 91.292934] [<81503735>] syscall_call+0x7/0xb
>
> Thanks,
> Fengguang


2012-08-22 07:54:50

by Feng Tang

[permalink] [raw]
Subject: Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

Hi Fengguang,


On Wed, 22 Aug 2012 10:50:08 +0800
Fengguang Wu <[email protected]> wrote:

> Feng,
>
> > 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 <[email protected]>
> AuthorDate: Wed May 30 23:15:41 2012 +0800
> Commit: Ingo Molnar <[email protected]>
> 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?

Thanks,
Feng

------------
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);
return pdev;
}



>
> > [ 91.282131] machine restart
> > [ 91.283895] ------------[ cut here ]------------
> > [ 91.284731] WARNING: at /c/wfg/linux/kernel/lockdep.c:2739 lockdep_trace_alloc+0x1fb/0x210()
> > [ 91.286132] Modules linked in:
> > [ 91.286703] Pid: 697, comm: reboot Not tainted 3.5.0-00024-g01ff5db-dirty #4
> > [ 91.287859] Call Trace:
> > [ 91.288289] [<81050148>] warn_slowpath_common+0xb8/0x100
> > [ 91.289338] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> > [ 91.290264] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> > [ 91.291161] [<810501ce>] warn_slowpath_null+0x3e/0x50
> > [ 91.292042] [<8110acdb>] lockdep_trace_alloc+0x1fb/0x210
> > [ 91.292934] [<81228e25>] kmem_cache_alloc_trace+0x55/0x600
> > [ 91.292934] [<813025ca>] ? kobject_put+0x9a/0x160
> > [ 91.292934] [<814e95e0>] ? klist_iter_exit+0x30/0x50
> > [ 91.292934] [<81405881>] ? bus_find_device+0xf1/0x120
> > [ 91.292934] [<81361a3c>] ? pci_get_subsys+0x11c/0x1b0
> > [ 91.292934] [<81361a3c>] pci_get_subsys+0x11c/0x1b0
> > [ 91.292934] [<81361afe>] pci_get_device+0x2e/0x40
> > [ 91.292934] [<81033e25>] mach_reboot_fixups+0xa5/0xd0
> > [ 91.292934] [<81027611>] native_machine_emergency_restart+0x1f1/0x590
> > [ 91.292934] [<814f2e00>] ? printk+0x4b/0x5b
> > [ 91.292934] [<810269ef>] native_machine_restart+0x6f/0x80
> > [ 91.292934] [<810271cc>] machine_restart+0x1c/0x30
> > [ 91.292934] [<810886e0>] kernel_restart+0x70/0xc0
> > [ 91.292934] [<81088a85>] sys_reboot+0x325/0x380
> > [ 91.292934] [<811f796c>] ? handle_pte_fault+0xdc/0x1740
> > [ 91.292934] [<811f93e7>] ? handle_mm_fault+0x417/0x4a0
> > [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> > [ 91.292934] [<810b33e7>] ? up_read+0x37/0x70
> > [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> > [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> > [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> > [ 91.292934] [<810b0270>] ? update_rmtp+0xe0/0xe0
> > [ 91.292934] [<8150376e>] ? restore_all+0xf/0xf
> > [ 91.292934] [<8103d880>] ? vmalloc_sync_all+0x320/0x320
> > [ 91.292934] [<81109fca>] ? trace_hardirqs_on_caller+0x28a/0x380
> > [ 91.292934] [<81311594>] ? trace_hardirqs_on_thunk+0xc/0x10
> > [ 91.292934] [<81503735>] syscall_call+0x7/0xb
> >
> > Thanks,
> > Fengguang

2012-08-22 13:03:54

by Fengguang Wu

[permalink] [raw]
Subject: Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

On Wed, Aug 22, 2012 at 03:49:08PM +0800, Tang, Feng wrote:
> Hi Fengguang,
>
>
> On Wed, 22 Aug 2012 10:50:08 +0800
> Fengguang Wu <[email protected]> wrote:
>
> > Feng,
> >
> > > 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 <[email protected]>
> > AuthorDate: Wed May 30 23:15:41 2012 +0800
> > Commit: Ingo Molnar <[email protected]>
> > 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?

Good idea! I like this simple solution. It will sure fix the warning.

Reviewed-by: Fengguang Wu <[email protected]>

Thanks,
Fengguang

> ------------
> 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);
> return pdev;
> }
>
>
>
> >
> > > [ 91.282131] machine restart
> > > [ 91.283895] ------------[ cut here ]------------
> > > [ 91.284731] WARNING: at /c/wfg/linux/kernel/lockdep.c:2739 lockdep_trace_alloc+0x1fb/0x210()
> > > [ 91.286132] Modules linked in:
> > > [ 91.286703] Pid: 697, comm: reboot Not tainted 3.5.0-00024-g01ff5db-dirty #4
> > > [ 91.287859] Call Trace:
> > > [ 91.288289] [<81050148>] warn_slowpath_common+0xb8/0x100
> > > [ 91.289338] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> > > [ 91.290264] [<8110acdb>] ? lockdep_trace_alloc+0x1fb/0x210
> > > [ 91.291161] [<810501ce>] warn_slowpath_null+0x3e/0x50
> > > [ 91.292042] [<8110acdb>] lockdep_trace_alloc+0x1fb/0x210
> > > [ 91.292934] [<81228e25>] kmem_cache_alloc_trace+0x55/0x600
> > > [ 91.292934] [<813025ca>] ? kobject_put+0x9a/0x160
> > > [ 91.292934] [<814e95e0>] ? klist_iter_exit+0x30/0x50
> > > [ 91.292934] [<81405881>] ? bus_find_device+0xf1/0x120
> > > [ 91.292934] [<81361a3c>] ? pci_get_subsys+0x11c/0x1b0
> > > [ 91.292934] [<81361a3c>] pci_get_subsys+0x11c/0x1b0
> > > [ 91.292934] [<81361afe>] pci_get_device+0x2e/0x40
> > > [ 91.292934] [<81033e25>] mach_reboot_fixups+0xa5/0xd0
> > > [ 91.292934] [<81027611>] native_machine_emergency_restart+0x1f1/0x590
> > > [ 91.292934] [<814f2e00>] ? printk+0x4b/0x5b
> > > [ 91.292934] [<810269ef>] native_machine_restart+0x6f/0x80
> > > [ 91.292934] [<810271cc>] machine_restart+0x1c/0x30
> > > [ 91.292934] [<810886e0>] kernel_restart+0x70/0xc0
> > > [ 91.292934] [<81088a85>] sys_reboot+0x325/0x380
> > > [ 91.292934] [<811f796c>] ? handle_pte_fault+0xdc/0x1740
> > > [ 91.292934] [<811f93e7>] ? handle_mm_fault+0x417/0x4a0
> > > [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> > > [ 91.292934] [<810b33e7>] ? up_read+0x37/0x70
> > > [ 91.292934] [<8103e07b>] ? do_page_fault+0x7fb/0xb30
> > > [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> > > [ 91.292934] [<8123c063>] ? do_sys_open+0x3a3/0x3f0
> > > [ 91.292934] [<810b0270>] ? update_rmtp+0xe0/0xe0
> > > [ 91.292934] [<8150376e>] ? restore_all+0xf/0xf
> > > [ 91.292934] [<8103d880>] ? vmalloc_sync_all+0x320/0x320
> > > [ 91.292934] [<81109fca>] ? trace_hardirqs_on_caller+0x28a/0x380
> > > [ 91.292934] [<81311594>] ? trace_hardirqs_on_thunk+0xc/0x10
> > > [ 91.292934] [<81503735>] syscall_call+0x7/0xb
> > >
> > > Thanks,
> > > Fengguang

2012-08-22 18:03:19

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

On Wed, Aug 22, 2012 at 12:49 AM, Feng Tang <[email protected]> wrote:
> Hi Fengguang,
>
>
> On Wed, 22 Aug 2012 10:50:08 +0800
> Fengguang Wu <[email protected]> wrote:
>
>> Feng,
>>
>> > 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 <[email protected]>
>> AuthorDate: Wed May 30 23:15:41 2012 +0800
>> Commit: Ingo Molnar <[email protected]>
>> 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?

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().

> ------------
> 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;
> }

2012-08-23 05:51:32

by Feng Tang

[permalink] [raw]
Subject: Re: pci_get_subsys: GFP_KERNEL allocations with IRQs disabled

Hi Bjorn,

On Wed, 22 Aug 2012 11:02:52 -0700
Bjorn Helgaas <[email protected]> wrote:

> On Wed, Aug 22, 2012 at 12:49 AM, Feng Tang <[email protected]> wrote:
> > Hi Fengguang,
> >
> >
> > On Wed, 22 Aug 2012 10:50:08 +0800
> > Fengguang Wu <[email protected]> wrote:
> >
> >> Feng,
> >>
> >> > 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 <[email protected]>
> >> AuthorDate: Wed May 30 23:15:41 2012 +0800
> >> Commit: Ingo Molnar <[email protected]>
> >> 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.

>
> > ------------
> > 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

2012-08-23 07:50:37

by Feng Tang

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

>From 57a28ee5e7662ca28ba4c793aa037d64bd082dee Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Wed, 22 Aug 2012 15:41:51 +0800
Subject: [PATCH 1/2] PCI: Use local parameter pci_device_id for pci_get_subsys/class()

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 casue 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 suggeted by Bjorn Helgaas

Bisected-by: Fengguang Wu <[email protected]>
Signed-off-by: Feng Tang <[email protected]>
Reviewed-by: Fengguang Wu <[email protected]>
---
drivers/pci/search.c | 35 +++++++++++------------------------
1 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 993d4a0..78a08b1 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -245,8 +245,7 @@ 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_dev *pdev;
- struct pci_device_id *id;
+ struct pci_device_id id;

/*
* pci_find_subsys() can be called on the ide_setup() path,
@@ -257,18 +256,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;

- return pdev;
+ return pci_get_dev_by_id(&id, from);
}

/**
@@ -307,19 +300,13 @@ 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_dev *dev;
- struct pci_device_id *id;
+ struct pci_device_id id;

- id = kzalloc(sizeof(*id), GFP_KERNEL);
- if (!id)
- return NULL;
- id->vendor = id->device = id->subvendor = id->subdevice = PCI_ANY_ID;
- id->class_mask = PCI_ANY_ID;
- id->class = class;
+ id.vendor = id.device = id.subvendor = id.subdevice = PCI_ANY_ID;
+ id.class_mask = PCI_ANY_ID;
+ id.class = class;

- dev = pci_get_dev_by_id(id, from);
- kfree(id);
- return dev;
+ return pci_get_dev_by_id(&id, from);
}

/**
--
1.7.1

2012-08-23 07:51:36

by Feng Tang

[permalink] [raw]
Subject: [PATCH 2/2] PCI: Remove the obsolete no_pci_devices() check

>From 9f2f3bbdf65f669e091c72b9648a4a0394ce28f5 Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Thu, 23 Aug 2012 14:55:48 +0800
Subject: [PATCH 2/2] PCI: Remove the obsolete no_pci_devices() check

In function pci_get_subsys() there is a check:

/*
* pci_find_subsys() can be called on the ide_setup() path,
* super-early in boot. But the down_read() will enable local
* interrupts, which can cause some machines to crash. So here we
* detect and flag that situation and bail out early.
*/
if (unlikely(no_pci_devices()))
return NULL;

But there is no ide_setup() now, and no down_read() either, which
makes the check absolete. So remove it.

Signed-off-by: Feng Tang <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/pci/search.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 78a08b1..e6e604f 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -247,15 +247,6 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
{
struct pci_device_id id;

- /*
- * pci_find_subsys() can be called on the ide_setup() path,
- * super-early in boot. But the down_read() will enable local
- * interrupts, which can cause some machines to crash. So here we
- * detect and flag that situation and bail out early.
- */
- if (unlikely(no_pci_devices()))
- return NULL;
-
id.vendor = vendor;
id.device = device;
id.subvendor = ss_vendor;
--
1.7.1