Hello.
On 12-04-2011 10:41, Roedel, Joerg wrote:
Hmm, I see how it can be done differently, but no real bug.
Never mind, you're right. In the no-race case, the initial count is
always going to be 0, and so incrementing info.probe_count is the right
thing to do.
Okay, cool. So now that we have settled on this I removed the last
occurence of '2.6.38' from the changelog and here is the result. No
code changes since v5.
I've pointed out the mistake in the code that you didn't fix... :-(
From a6e00d237f20e940afdbcaafb27117fe8730466f Mon Sep 17 00:00:00 2001
From: Joerg Roedel<joerg.roedel@xxxxxxx>
Date: Wed, 6 Apr 2011 13:07:53 +0200
Subject: [PATCH] USB host: Fix lockdep warning in AMD PLL quirk
Booting latest kernel on my test machine produces a lockdep
warning from the usb_amd_find_chipset_info() function:
WARNING: at /data/lemmy/linux.trees.git/kernel/lockdep.c:2465 lockdep_trace_alloc+0x95/0xc2()
Hardware name: Snook
Modules linked in:
Pid: 959, comm: work_for_cpu Not tainted 2.6.39-rc2+ #22
Call Trace:
[<ffffffff8103c0d4>] warn_slowpath_common+0x80/0x98
[<ffffffff812387e6>] ? T.492+0x24/0x26
[<ffffffff8103c101>] warn_slowpath_null+0x15/0x17
[<ffffffff81068667>] lockdep_trace_alloc+0x95/0xc2
[<ffffffff810ed9ac>] slab_pre_alloc_hook+0x18/0x3b
[<ffffffff810ef227>] kmem_cache_alloc_trace+0x25/0xba
[<ffffffff812387e6>] T.492+0x24/0x26
[<ffffffff81238816>] pci_get_subsys+0x2e/0x73
[<ffffffff8123886c>] pci_get_device+0x11/0x13
[<ffffffff814082a9>] usb_amd_find_chipset_info+0x3f/0x18a
...
It turns out that this function calls pci_get_device under a spin_lock
with irqs disabled, but the pci_get_device function is only allowed in
preemptible context.
This patch fixes the warning by making all data-structure
modifications on temporal storage and commiting this back
into the visible structure at the end. While at it, this
patch also moves the pci_dev_put calls out of the spinlocks
because this function might sleep too.
Signed-off-by: Joerg Roedel <joerg.roedel@xxxxxxx>
---
drivers/usb/host/pci-quirks.c | 117 ++++++++++++++++++++++++++---------------
1 files changed, 74 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 1d586d4..c6eb69c 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
[...]
@@ -294,20 +322,23 @@ void usb_amd_dev_put(void)
return;
}
- if (amd_chipset.nb_dev) {
- pci_dev_put(amd_chipset.nb_dev);
- amd_chipset.nb_dev = NULL;
- }
- if (amd_chipset.smbus_dev) {
- pci_dev_put(amd_chipset.smbus_dev);
- amd_chipset.smbus_dev = NULL;
- }
+ /* save them to pci_dev_put outside of spinlock */
+ nb = amd_chipset.nb_dev;
+ smbus = amd_chipset.smbus_dev;
+
+ amd_chipset.nb_dev = NULL;
+ amd_chipset.smbus_dev = NULL;
amd_chipset.nb_type = 0;
amd_chipset.sb_type = 0;
amd_chipset.isoc_reqs = 0;
amd_chipset.probe_result = 0;
spin_unlock_irqrestore(&amd_lock, flags);
+
+ if (nb)
+ pci_dev_put(nb);
+ if (smbus)
+ pci_dev_put(amd_chipset.smbus_dev);
Here it is. This pointer is NULL, it should be 'smbus' instead.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
