from Jie Yang <[email protected]>
On Saturday, August 30, 2008 5:27 AM
Andrew Morton <[email protected]]>
> On Fri, 29 Aug 2008 14:08:09 -0700 (PDT)
> [email protected] wrote:
>
> > http://bugzilla.kernel.org/show_bug.cgi?id=11454
> >
> > Summary: atl1e - BUG: scheduling while atomic:
> > modprobe/678/0x00000002
> > Product: Drivers
> > Version: 2.5
> > KernelVersion: 2.6.27-rc5
> > Platform: All
> > OS/Version: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: high
> > Priority: P1
> > Component: Network
> > AssignedTo: [email protected]
> > ReportedBy: [email protected]
> >
> >
> > Latest working kernel version: -
> > Earliest failing kernel version: 2.6.27-rc1
> > Software: x86_64
> >
>
> drivers/net/atl1e/atl1e_hw.c: struct atl1e_adapter *adapter
> = (struct atl1e_adapter *)hw->adapter;
> drivers/net/atl1e/atl1e_hw.c: struct atl1e_adapter *adapter
> = (struct atl1e_adapter *)hw->adapter;
> drivers/net/atl1e/atl1e_hw.c: struct atl1e_adapter *adapter
> = (struct atl1e_adapter *)hw->adapter;
>
> are unneeded and undesirable. hw->adapter already has type
> atl1e_adapter*.
>
just as Matthew Wilcox <[email protected]> mentioned:
> 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.
So, just do not take mdio_lock lock in atl1e_probe, and remove the
unneeded (struct atl1e_adapter *)
Signed-off-by: Jie Yang <[email protected]>
---
BTW: I do not know if this format is suitable for repling [Bugme-new],
if it is not suitable, just let me know.
diff --git a/drivers/net/atl1e/atl1e_hw.c b/drivers/net/atl1e/atl1e_hw.c
index 949e753..8cbc1b5 100644
--- a/drivers/net/atl1e/atl1e_hw.c
+++ b/drivers/net/atl1e/atl1e_hw.c
@@ -397,7 +397,7 @@ static int atl1e_phy_setup_autoneg_adv(struct atl1e_hw *hw)
*/
int atl1e_phy_commit(struct atl1e_hw *hw)
{
- struct atl1e_adapter *adapter = (struct atl1e_adapter *)hw->adapter;
+ struct atl1e_adapter *adapter = hw->adapter;
struct pci_dev *pdev = adapter->pdev;
int ret_val;
u16 phy_data;
@@ -431,7 +431,7 @@ int atl1e_phy_commit(struct atl1e_hw *hw)
int atl1e_phy_init(struct atl1e_hw *hw)
{
- struct atl1e_adapter *adapter = (struct atl1e_adapter *)hw->adapter;
+ struct atl1e_adapter *adapter = hw->adapter;
struct pci_dev *pdev = adapter->pdev;
s32 ret_val;
u16 phy_val;
@@ -525,7 +525,7 @@ int atl1e_phy_init(struct atl1e_hw *hw)
*/
int atl1e_reset_hw(struct atl1e_hw *hw)
{
- struct atl1e_adapter *adapter = (struct atl1e_adapter *)hw->adapter;
+ struct atl1e_adapter *adapter = hw->adapter;
struct pci_dev *pdev = adapter->pdev;
u32 idle_status_data = 0;
diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
index 7685b99..9b60352 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -2390,9 +2390,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);
On Sun, Aug 31, 2008 at 12:51:58PM +0800, [email protected] wrote:
> from Jie Yang <[email protected]>
>
> On Saturday, August 30, 2008 5:27 AM
> Andrew Morton <[email protected]]>
> > On Fri, 29 Aug 2008 14:08:09 -0700 (PDT)
> > [email protected] wrote:
> >
> > > http://bugzilla.kernel.org/show_bug.cgi?id=11454
> > >
> > > Summary: atl1e - BUG: scheduling while atomic:
> > > modprobe/678/0x00000002
> > > Product: Drivers
> > > Version: 2.5
> > > KernelVersion: 2.6.27-rc5
> > > Platform: All
> > > OS/Version: Linux
> > > Tree: Mainline
> > > Status: NEW
> > > Severity: high
> > > Priority: P1
> > > Component: Network
> > > AssignedTo: [email protected]
How about taking my original patch, sent August 12th instead? That has
a good changelog and proper attribution. The removal of the
unnecessary casts can be a separate message.
If you weren't cc'd on the patch (Jeff was), you can pick it up from
netdev.
--
Matthew Wilcox Intel Open Source Technology Centre
"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 Sunday, August 31, 2008 1:09 PM
Matthew Wilcox <[email protected]> wrote:
> How about taking my original patch, sent August 12th instead?
> That has a good changelog and proper attribution. The
> removal of the unnecessary casts can be a separate message.
>
> If you weren't cc'd on the patch (Jeff was), you can pick it
> up from netdev.
>
Yes, I do have the patch, should I resend with Signed-off-by: Matthew Wilcox <[email protected]>.
for that patch may have conflicts now.
the origin patch:
diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c index 82d7be1..ba22a51 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -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);
the new one:
diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
index 7685b99..9b60352 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -2390,9 +2390,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);
the line num changed from 2389 to 2390. :(
Best wishes
jie