* Re: [ACPI/PCI] possible recursive locking detected
[not found] ` <CAE9FiQVQJQfBZn7AH4M5hibx=iNa9RbfSaBdb35FWxDiRXHvrA@mail.gmail.com>
@ 2012-09-22 7:04 ` Yinghai Lu
2012-09-22 8:10 ` Fengguang Wu
0 siblings, 1 reply; 4+ messages in thread
From: Yinghai Lu @ 2012-09-22 7:04 UTC (permalink / raw)
To: Fengguang Wu, Len Brown, Bjorn Helgaas
Cc: Taku Izumi, LKML, ACPI Devel Maling List, linux-pci
[-- Attachment #1: Type: text/plain, Size: 5283 bytes --]
On Fri, Sep 21, 2012 at 9:53 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Sep 21, 2012 at 5:35 PM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>> Hi Taku,
>>
>> The below oops is pretty reproducible, and shows up first in:
>>
>> tree: git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/taku-acpi-pci-host-bridge-v3
>> head: e3faec8ea9c8aa683c56fa20ff2c58a4c5857960
>> commit: d3c663236318a43fed5d86a643e6ea2534e9220e [5/7] PCI/ACPI: Protect acpi_pci_roots list with mutex
>>
>> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
>> [ 8.613239] lock(acpi_pci_root_lock);
>> [ 8.613239] lock(acpi_pci_root_lock);
>> [ 8.613239] #0: (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
>>
>> [ 8.610859]
>> [ 8.611385] =============================================
>> [ 8.612505] [ INFO: possible recursive locking detected ]
>> [ 8.613239] 3.6.0-rc1-00022-gd3c6632 #7512 Not tainted
>> [ 8.613239] ---------------------------------------------
>> [ 8.613239] swapper/0/1 is trying to acquire lock:
>> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239]
>> [ 8.613239] but task is already holding lock:
>> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
>> [ 8.613239]
>> [ 8.613239] other info that might help us debug this:
>> [ 8.613239] Possible unsafe locking scenario:
>> [ 8.613239]
>> [ 8.613239] CPU0
>> [ 8.613239] ----
>> [ 8.613239] lock(acpi_pci_root_lock);
>> [ 8.613239] lock(acpi_pci_root_lock);
>> [ 8.613239]
>> [ 8.613239] *** DEADLOCK ***
>> [ 8.613239]
>> [ 8.613239] May be due to missing lock nesting notation
>> [ 8.613239]
>> [ 8.613239] 1 lock held by swapper/0/1:
>> [ 8.613239] #0: (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
>> [ 8.613239]
>> [ 8.613239] stack backtrace:
>> [ 8.613239] Pid: 1, comm: swapper/0 Not tainted 3.6.0-rc1-00022-gd3c6632 #7512
>> [ 8.613239] Call Trace:
>> [ 8.613239] [<ffffffff810d7ae5>] __lock_acquire+0xbef/0xd04
>> [ 8.613239] [<ffffffff8106e60b>] ? kvm_clock_read+0x2e/0x36
>> [ 8.613239] [<ffffffff810d80a3>] lock_acquire+0xd5/0x119
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff8270c01b>] __mutex_lock_common+0x58/0x387
>> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff8106e60b>] ? kvm_clock_read+0x2e/0x36
>> [ 8.613239] [<ffffffff81118ea4>] ? irq_trace+0x14/0x21
>> [ 8.613239] [<ffffffff8270c459>] mutex_lock_nested+0x40/0x45
>> [ 8.613239] [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
>> [ 8.613239] [<ffffffff816beb25>] acpi_pci_get_bridge_handle+0x2c/0x2e
>> [ 8.613239] [<ffffffff816beb3f>] acpi_pci_check_ejectable+0x18/0x49
>> [ 8.613239] [<ffffffff8270ea46>] ? _raw_spin_unlock_irqrestore+0x45/0x61
>> [ 8.613239] [<ffffffff816c6981>] register_slot+0x2f/0x460
>> [ 8.613239] [<ffffffff810afbf8>] ? up+0x39/0x3e
>> [ 8.613239] [<ffffffff81757575>] acpi_ns_walk_namespace+0xbe/0x179
>> [ 8.613239] [<ffffffff8173c09a>] ? acpi_os_wait_semaphore+0x45/0x5a
>> [ 8.613239] [<ffffffff816c6952>] ? kzalloc.constprop.14+0x10/0x10
>> [ 8.613239] [<ffffffff816c6952>] ? kzalloc.constprop.14+0x10/0x10
>> [ 8.613239] [<ffffffff81757a1d>] acpi_walk_namespace+0x98/0xcb
>> [ 8.613239] [<ffffffff816c6dfe>] init_bridge_misc+0x4c/0xd5
>> [ 8.613239] [<ffffffff816c6f70>] add_bridge+0xe9/0x138
>> [ 8.613239] [<ffffffff817423c9>] acpi_pci_register_driver+0x52/0x79
>> [ 8.613239] [<ffffffff838ce2b2>] ? shpcd_init+0xf0/0xf0
>> [ 8.613239] [<ffffffff838ce34a>] acpiphp_glue_init+0x48/0x51
>> [ 8.613239] [<ffffffff838ce2dd>] acpiphp_init+0x2b/0x50
>> [ 8.613239] [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
>> [ 8.613239] [<ffffffff8389ccef>] kernel_init+0x13c/0x1c0
>> [ 8.613239] [<ffffffff8389c590>] ? do_early_param+0x8c/0x8c
>> [ 8.613239] [<ffffffff82716944>] kernel_thread_helper+0x4/0x10
>> [ 8.613239] [<ffffffff8270ee70>] ? retint_restore_args+0x13/0x13
>> [ 8.613239] [<ffffffff8389cbb3>] ? start_kernel+0x3d1/0x3d1
>> [ 8.613239] [<ffffffff82716940>] ? gs_change+0x13/0x13
>
> Please check attached patch that should fix the problem.
updated more aggressive version. two patches.
-Yinghai
[-- Attachment #2: pci_root_bus_to_handle_1.patch --]
[-- Type: application/octet-stream, Size: 2137 bytes --]
Subject: [PATCH] PCI, ACPI: Use pci_host_bridge dev to save acpi_handle
So we could avoid calling acpi_get_pci_rootbridge_handle that could cause deadlock.
non root pci bus is using bridge->dev archdata.acpi_handle.
for root bus that field is not used, could just save root handle, and get that
later when need to get root bus's handle.
-v2: move setting code to pci_root.c
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
drivers/acpi/pci_root.c | 21 +++------------------
drivers/pci/pci-acpi.c | 4 +++-
include/acpi/acpi_bus.h | 1 -
include/linux/pci-acpi.h | 15 ++++++++++-----
4 files changed, 16 insertions(+), 25 deletions(-)
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
--- linux-2.6.orig/include/linux/pci-acpi.h
+++ linux-2.6/include/linux/pci-acpi.h
@@ -22,19 +22,24 @@ extern phys_addr_t acpi_pci_root_get_mcf
static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
{
struct pci_bus *pbus = pdev->bus;
+
/* Find a PCI root bus */
while (!pci_is_root_bus(pbus))
pbus = pbus->parent;
- return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
- pbus->number);
+
+ return DEVICE_ACPI_HANDLE(pbus->bridge);
}
static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
{
+ struct device *dev;
+
if (!pci_is_root_bus(pbus))
- return DEVICE_ACPI_HANDLE(&(pbus->self->dev));
- return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
- pbus->number);
+ dev = &pbus->self->dev;
+ else
+ dev = pbus->bridge;
+
+ return DEVICE_ACPI_HANDLE(dev);
}
#endif
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -542,6 +524,9 @@ static int __devinit acpi_pci_root_add(s
goto out_del_root;
}
+ /* to find handle from root bus */
+ root->bus->bridge->archdata.acpi_handle = device->handle;
+
/*
* Attach ACPI-PCI Context
* -----------------------
[-- Attachment #3: pci_root_bus_to_handle_2.patch --]
[-- Type: application/octet-stream, Size: 2361 bytes --]
Subject: [PATCH] PCI, ACPI: kill acpi_get_pci_rootbridge_handle
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
drivers/acpi/pci_root.c | 21 +++------------------
drivers/pci/pci-acpi.c | 4 +++-
include/acpi/acpi_bus.h | 1 -
include/linux/pci-acpi.h | 15 ++++++++++-----
4 files changed, 16 insertions(+), 25 deletions(-)
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -110,24 +110,6 @@ void acpi_pci_unregister_driver(struct a
}
EXPORT_SYMBOL(acpi_pci_unregister_driver);
-acpi_handle acpi_get_pci_rootbridge_handle(unsigned int seg, unsigned int bus)
-{
- struct acpi_pci_root *root;
- acpi_handle handle = NULL;
-
- mutex_lock(&acpi_pci_root_lock);
- list_for_each_entry(root, &acpi_pci_roots, node)
- if ((root->segment == (u16) seg) &&
- (root->secondary.start == (u16) bus)) {
- handle = root->device->handle;
- break;
- }
- mutex_unlock(&acpi_pci_root_lock);
- return handle;
-}
-
-EXPORT_SYMBOL_GPL(acpi_get_pci_rootbridge_handle);
-
/**
* acpi_is_root_bridge - determine whether an ACPI CA node is a PCI root bridge
* @handle - the ACPI CA node in question.
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -372,9 +372,11 @@ static int acpi_pci_find_root_bridge(str
num = sscanf(dev_name(dev), "pci%04x:%02x", &seg, &bus);
if (num != 2)
return -ENODEV;
- *handle = acpi_get_pci_rootbridge_handle(seg, bus);
+
+ *handle = DEVICE_ACPI_HANDLE(dev);
if (!*handle)
return -ENODEV;
+
return 0;
}
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -413,7 +413,6 @@ struct acpi_pci_root {
/* helper */
acpi_handle acpi_get_child(acpi_handle, u64);
int acpi_is_root_bridge(acpi_handle);
-acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ACPI/PCI] possible recursive locking detected
2012-09-22 7:04 ` [ACPI/PCI] possible recursive locking detected Yinghai Lu
@ 2012-09-22 8:10 ` Fengguang Wu
2012-09-22 14:11 ` Yinghai Lu
0 siblings, 1 reply; 4+ messages in thread
From: Fengguang Wu @ 2012-09-22 8:10 UTC (permalink / raw)
To: Yinghai Lu
Cc: Len Brown, Bjorn Helgaas, Taku Izumi, LKML,
ACPI Devel Maling List, linux-pci
On Sat, Sep 22, 2012 at 12:04:51AM -0700, Yinghai Lu wrote:
> On Fri, Sep 21, 2012 at 9:53 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Fri, Sep 21, 2012 at 5:35 PM, Fengguang Wu <fengguang.wu@intel.com> wrote:
> >> Hi Taku,
> >>
> >> The below oops is pretty reproducible, and shows up first in:
> >>
> >> tree: git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/taku-acpi-pci-host-bridge-v3
> >> head: e3faec8ea9c8aa683c56fa20ff2c58a4c5857960
> >> commit: d3c663236318a43fed5d86a643e6ea2534e9220e [5/7] PCI/ACPI: Protect acpi_pci_roots list with mutex
> >>
> >> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
> >> [ 8.613239] lock(acpi_pci_root_lock);
> >> [ 8.613239] lock(acpi_pci_root_lock);
> >> [ 8.613239] #0: (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
> >> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239] [<ffffffff81741e5f>] ? acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239] [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
> >>
> >> [ 8.610859]
> >> [ 8.611385] =============================================
> >> [ 8.612505] [ INFO: possible recursive locking detected ]
> >> [ 8.613239] 3.6.0-rc1-00022-gd3c6632 #7512 Not tainted
> >> [ 8.613239] ---------------------------------------------
> >> [ 8.613239] swapper/0/1 is trying to acquire lock:
> >> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81741e5f>] acpi_get_pci_rootbridge_handle+0x22/0x63
> >> [ 8.613239]
> >> [ 8.613239] but task is already holding lock:
> >> [ 8.613239] (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
> >> [ 8.613239]
> >> [ 8.613239] other info that might help us debug this:
> >> [ 8.613239] Possible unsafe locking scenario:
> >> [ 8.613239]
> >> [ 8.613239] CPU0
> >> [ 8.613239] ----
> >> [ 8.613239] lock(acpi_pci_root_lock);
> >> [ 8.613239] lock(acpi_pci_root_lock);
> >> [ 8.613239]
> >> [ 8.613239] *** DEADLOCK ***
> >> [ 8.613239]
> >> [ 8.613239] May be due to missing lock nesting notation
> >> [ 8.613239]
> >> [ 8.613239] 1 lock held by swapper/0/1:
> >> [ 8.613239] #0: (acpi_pci_root_lock){+.+.+.}, at: [<ffffffff81742398>] acpi_pci_register_driver+0x21/0x79
> >> [ 8.613239]
> > Please check attached patch that should fix the problem.
>
> updated more aggressive version. two patches.
Yes they work nicely. Thank you very much!
Thanks,
Fengguang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ACPI/PCI] possible recursive locking detected
2012-09-22 8:10 ` Fengguang Wu
@ 2012-09-22 14:11 ` Yinghai Lu
2012-09-24 22:38 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Yinghai Lu @ 2012-09-22 14:11 UTC (permalink / raw)
To: Fengguang Wu, Bjorn Helgaas
Cc: Len Brown, Taku Izumi, LKML, ACPI Devel Maling List, linux-pci
[-- Attachment #1: Type: text/plain, Size: 367 bytes --]
On Sat, Sep 22, 2012 at 1:10 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>
>> > Please check attached patch that should fix the problem.
>>
>> updated more aggressive version. two patches.
>
> Yes they work nicely. Thank you very much!
Thanks.
v2 breaks acpi_bind_one.
So we should use attached -v3.
Bjorn, please apply -v3 before Taku's patchset.
-Yinghai
[-- Attachment #2: pci_root_bus_to_handle_v3.patch --]
[-- Type: application/octet-stream, Size: 1869 bytes --]
Subject: [PATCH -v3] PCI, ACPI: Use DEVICE_ACPI_HANDLE for root bus struct pointer
So we could avoid calling acpi_get_pci_rootbridge_handle that could cause deadlock.
non root pci bus is using bridge->dev archdata.acpi_handle.
we shoudl use that same field for for root bus
-v3: do not need to and should not set that archdata.acpi_handle directly
acpi_pci_root_add
==> pci_acpi_scan_root
==> pci_create_root_bus
==> device_register(&bridge->dev)
==> device_add
==> platform_notify
==> acpi_bind_one
will set that for us.
that mean's we still need to keep acpi_get_pci_rootbridge_handle for
acpi_pci_find_root_bridge in acpi_pci_bus bus_type.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
include/linux/pci-acpi.h | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
--- linux-2.6.orig/include/linux/pci-acpi.h
+++ linux-2.6/include/linux/pci-acpi.h
@@ -22,19 +22,24 @@ extern phys_addr_t acpi_pci_root_get_mcf
static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
{
struct pci_bus *pbus = pdev->bus;
+
/* Find a PCI root bus */
while (!pci_is_root_bus(pbus))
pbus = pbus->parent;
- return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
- pbus->number);
+
+ return DEVICE_ACPI_HANDLE(pbus->bridge);
}
static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
{
+ struct device *dev;
+
if (!pci_is_root_bus(pbus))
- return DEVICE_ACPI_HANDLE(&(pbus->self->dev));
- return acpi_get_pci_rootbridge_handle(pci_domain_nr(pbus),
- pbus->number);
+ dev = &pbus->self->dev;
+ else
+ dev = pbus->bridge;
+
+ return DEVICE_ACPI_HANDLE(dev);
}
#endif
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ACPI/PCI] possible recursive locking detected
2012-09-22 14:11 ` Yinghai Lu
@ 2012-09-24 22:38 ` Bjorn Helgaas
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2012-09-24 22:38 UTC (permalink / raw)
To: Yinghai Lu
Cc: Fengguang Wu, Len Brown, Taku Izumi, LKML, ACPI Devel Maling List,
linux-pci
On Sat, Sep 22, 2012 at 8:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sat, Sep 22, 2012 at 1:10 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>>
>>> > Please check attached patch that should fix the problem.
>>>
>>> updated more aggressive version. two patches.
>>
>> Yes they work nicely. Thank you very much!
>
> Thanks.
>
> v2 breaks acpi_bind_one.
>
> So we should use attached -v3.
>
> Bjorn, please apply -v3 before Taku's patchset.
I applied this as the first patch in
pci/taku-acpi-pci-host-bridge-v3. I also merged that branch to
pci/next.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-24 22:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20120922003557.GF5959@localhost>
[not found] ` <CAE9FiQVQJQfBZn7AH4M5hibx=iNa9RbfSaBdb35FWxDiRXHvrA@mail.gmail.com>
2012-09-22 7:04 ` [ACPI/PCI] possible recursive locking detected Yinghai Lu
2012-09-22 8:10 ` Fengguang Wu
2012-09-22 14:11 ` Yinghai Lu
2012-09-24 22:38 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).