Don't take the mdio_lock in atl1e_probe
Subsystems (2)
PATCH 1/1 Don't take the mdio_lock in atl1e_probe
Lockdep warns about the mdio_lock taken with interrupts enabled then later taken from interrupt context. Initially, I considered changing these to spin_lock_irq/spin_unlock_irq, but then I looked at atl1e_phy_init() and saw that it calls msleep(). Sleeping while holding a spinlock is not allowed either.
In the probe path, we haven't registered the interrupt handler, so it can't poke at this card yet. It's before we call register_netdev(), so I don't think any other threads can reach this card either. If I'm right, we don't need a spinlock at all.
drivers/net/atl1e/atl1e_main.c +0 -2
@@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct pci_dev *pdev, }/* Init GPHY as early as possible due to power saving issue */ -spin_lock (&adapter->mdio_lock);atl1e_phy_init (&adapter->hw); -spin_unlock (&adapter->mdio_lock);/* reset the controller to * put the device in a known good starting state */ err =atl1e_reset_hw (&adapter->hw);
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
On Tuesday, August 12, 2008 9:13 PM
Matthew Wilcox <[email protected]> wrote
Quoted 25 lines
Lockdep warns about the
mdio_locktaken with interrupts enabled then later taken from interrupt context. Initially, I considered changing these to spin_lock_irq/spin_unlock_irq, but then I looked atatl1e_phy_init()and saw that it callsmsleep(). Sleeping while holding a spinlock is not allowed either.In the probe path, we haven't registered the interrupt handler, so it can't poke at this card yet. It's before we call
register_netdev(), so I don't think any other threads can reach this card either. If I'm right, we don't need a spinlock at all.Signed-off-by Matthew Wilcox <[email protected]>(quoted diff) +0
-0
b/drivers/net/atl1e/atl1e_main.cindex 82d7be1..ba22a51 100644drivers/net/atl1e/atl1e_main.c +0
-0@@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct pci_dev *pdev, }/* Init GPHY as early as possible due to power saving issue */
drivers/net/atl1e/atl1e_main.c static int __devinit atl1e_probe(struct pci_dev *pdev, · line 2395
Quoted 6 lines
drivers/net/atl1e/atl1e_main.c +0
-2-spin_lock (&adapter->mdio_lock);atl1e_phy_init (&adapter->hw); -spin_unlock (&adapter->mdio_lock);/* reset the controller to * put the device in a known good starting state */ err =atl1e_reset_hw (&adapter->hw);
Ok.
Thanks Matthew Wilcox
Jeff Garzik
AppliedQuoted 18 lines · Matthew Wilcox
Lockdep warns about the
mdio_locktaken with interrupts enabled then later taken from interrupt context. Initially, I considered changing these to spin_lock_irq/spin_unlock_irq, but then I looked atatl1e_phy_init()and saw that it callsmsleep(). Sleeping while holding a spinlock is not allowed either.In the probe path, we haven't registered the interrupt handler, so it can't poke at this card yet. It's before we call
register_netdev(), so I don't think any other threads can reach this card either. If I'm right, we don't need a spinlock at all.Signed-off-by Matthew Wilcox <[email protected]>drivers/net/atl1e/atl1e_main.c +0
-0
drivers/net/atl1e/atl1e_main.c static int __devinit atl1e_probe(struct pci_dev *pdev, · line 2394
Quoted 8 lines
drivers/net/atl1e/atl1e_main.c +0
-2@@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct pci_dev *pdev, }/* Init GPHY as early as possible due to power saving issue */ -spin_lock (&adapter->mdio_lock);atl1e_phy_init (&adapter->hw); -spin_unlock (&adapter->mdio_lock);/* reset the controller to * put the device in a known good starting state */
applied