BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
Call Trace:
show_trace
dump_stack
trace_hardirqs_on
:forcedeth:nv_nic_irq_other
handle_IRQ_event
__do_IRQ
do_IRQ
ret_from_intr
DWARF2 barf
default_idle
cpu_idle
rest_init
start_kernel
_sinittext
These 3 functions nv_nic_irq_tx(), nv_nic_irq_rx() and nv_nic_irq_other()
are reachable from IRQ context and process context. Make use of the
irq-save/restore spinlock variant.
(Compile tested only, since I do not have the hardware)
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: Jeff Garzik <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Dave Jones <[email protected]>
Cc: Andrew Morton <[email protected]>
---
drivers/net/forcedeth.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
Index: linux-2.6-mm/drivers/net/forcedeth.c
===================================================================
--- linux-2.6-mm.orig/drivers/net/forcedeth.c
+++ linux-2.6-mm/drivers/net/forcedeth.c
@@ -2497,6 +2497,7 @@ static irqreturn_t nv_nic_irq_tx(int foo
u8 __iomem *base = get_hwbase(dev);
u32 events;
int i;
+ unsigned long flags;
dprintk(KERN_DEBUG "%s: nv_nic_irq_tx\n", dev->name);
@@ -2508,16 +2509,16 @@ static irqreturn_t nv_nic_irq_tx(int foo
if (!(events & np->irqmask))
break;
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
nv_tx_done(dev);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
if (events & (NVREG_IRQ_TX_ERR)) {
dprintk(KERN_DEBUG "%s: received irq with events 0x%x. Probably TX fail.\n",
dev->name, events);
}
if (i > max_interrupt_work) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
/* disable interrupts on the nic */
writel(NVREG_IRQ_TX_ALL, base + NvRegIrqMask);
pci_push(base);
@@ -2527,7 +2528,7 @@ static irqreturn_t nv_nic_irq_tx(int foo
mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
}
printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_tx.\n", dev->name, i);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
break;
}
@@ -2601,6 +2602,7 @@ static irqreturn_t nv_nic_irq_rx(int foo
u8 __iomem *base = get_hwbase(dev);
u32 events;
int i;
+ unsigned long flags;
dprintk(KERN_DEBUG "%s: nv_nic_irq_rx\n", dev->name);
@@ -2614,14 +2616,14 @@ static irqreturn_t nv_nic_irq_rx(int foo
nv_rx_process(dev, dev->weight);
if (nv_alloc_rx(dev)) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
if (!np->in_shutdown)
mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
}
if (i > max_interrupt_work) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
/* disable interrupts on the nic */
writel(NVREG_IRQ_RX_ALL, base + NvRegIrqMask);
pci_push(base);
@@ -2631,7 +2633,7 @@ static irqreturn_t nv_nic_irq_rx(int foo
mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
}
printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_rx.\n", dev->name, i);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
break;
}
}
@@ -2648,6 +2650,7 @@ static irqreturn_t nv_nic_irq_other(int
u8 __iomem *base = get_hwbase(dev);
u32 events;
int i;
+ unsigned long flags;
dprintk(KERN_DEBUG "%s: nv_nic_irq_other\n", dev->name);
@@ -2660,14 +2663,14 @@ static irqreturn_t nv_nic_irq_other(int
break;
if (events & NVREG_IRQ_LINK) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
nv_link_irq(dev);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
}
if (np->need_linktimer && time_after(jiffies, np->link_timeout)) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
nv_linkchange(dev);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
np->link_timeout = jiffies + LINK_TIMEOUT;
}
if (events & (NVREG_IRQ_UNKNOWN)) {
@@ -2675,7 +2678,7 @@ static irqreturn_t nv_nic_irq_other(int
dev->name, events);
}
if (i > max_interrupt_work) {
- spin_lock_irq(&np->lock);
+ spin_lock_irqsave(&np->lock, flags);
/* disable interrupts on the nic */
writel(NVREG_IRQ_OTHER, base + NvRegIrqMask);
pci_push(base);
@@ -2685,7 +2688,7 @@ static irqreturn_t nv_nic_irq_other(int
mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
}
printk(KERN_DEBUG "%s: too many iterations (%d) in nv_nic_irq_other.\n", dev->name, i);
- spin_unlock_irq(&np->lock);
+ spin_unlock_irqrestore(&np->lock, flags);
break;
}
On Tue, 19 Sep 2006 14:55:22 +0200
Peter Zijlstra <[email protected]> wrote:
> BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
I wonder what line that was. DEBUG_LOCKS_WARN_ON(current->hardirq_context),
I suppose.
> Call Trace:
> show_trace
> dump_stack
> trace_hardirqs_on
> :forcedeth:nv_nic_irq_other
> handle_IRQ_event
> __do_IRQ
> do_IRQ
> ret_from_intr
> DWARF2 barf
It's good, isn't it?
> default_idle
> cpu_idle
> rest_init
> start_kernel
> _sinittext
>
> These 3 functions nv_nic_irq_tx(), nv_nic_irq_rx() and nv_nic_irq_other()
> are reachable from IRQ context and process context.
That's "fix deadlock", not "fix lockdep warning". However it's often the
case that it's not really deadlockable because (often) the card's IRQ line
has been disabled. That appears to be the case in nv_do_nic_poll()'s call
to nv_nic_irq_tx, for example.
> Make use of the
> irq-save/restore spinlock variant.
So this perhaps is another lockdep workaround..
On Tue, Sep 19, 2006 at 11:14:48AM -0700, Andrew Morton wrote:
> On Tue, 19 Sep 2006 14:55:22 +0200
> Peter Zijlstra <[email protected]> wrote:
>
> > BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
>
> I wonder what line that was. DEBUG_LOCKS_WARN_ON(current->hardirq_context),
> I suppose.
That's what it matches up to in the Fedora kernel. (We have a bunch of lockdep
fixes scooped up from lkml over the last month or so, which may offset us).
Dave
On Tue, 2006-09-19 at 11:14 -0700, Andrew Morton wrote:
> On Tue, 19 Sep 2006 14:55:22 +0200
> Peter Zijlstra <[email protected]> wrote:
>
> > BUG: warning at kernel/lockdep.c:1816/trace_hardirqs_on() (Not tainted)
>
> I wonder what line that was. DEBUG_LOCKS_WARN_ON(current->hardirq_context),
> I suppose.
Correct indeed.
> > Call Trace:
> > show_trace
> > dump_stack
> > trace_hardirqs_on
> > :forcedeth:nv_nic_irq_other
> > handle_IRQ_event
> > __do_IRQ
> > do_IRQ
> > ret_from_intr
> > DWARF2 barf
>
> It's good, isn't it?
Yeah, I hope we'll get it sorted out quickly....
> > default_idle
> > cpu_idle
> > rest_init
> > start_kernel
> > _sinittext
> >
> > These 3 functions nv_nic_irq_tx(), nv_nic_irq_rx() and nv_nic_irq_other()
> > are reachable from IRQ context and process context.
>
> That's "fix deadlock", not "fix lockdep warning". However it's often the
> case that it's not really deadlockable because (often) the card's IRQ line
> has been disabled. That appears to be the case in nv_do_nic_poll()'s call
> to nv_nic_irq_tx, for example.
Yeah, I saw some of that. But I'm not well versed in the various netdev
irq receive paths nor in the driver. I couldn't find the actual
maintainer in the MAINTAINERS file nor in the source, thanks for
including him in the CC. Lets hope he can say if this is an actual fix
or just a workaround.
> That's "fix deadlock", not "fix lockdep warning". However it's often the
> case that it's not really deadlockable because (often) the card's IRQ line
> has been disabled. That appears to be the case in nv_do_nic_poll()'s call
> to nv_nic_irq_tx, for example.
>
except when it's a shared irq line, then the game rules changed ;)