* [PATCH]: use spin_lock_irqsave in try_one_irq()
@ 2009-11-03 13:45 Prarit Bhargava
2009-11-03 14:58 ` Yong Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Prarit Bhargava @ 2009-11-03 13:45 UTC (permalink / raw)
To: linux-kernel, ebiederm, akpm; +Cc: Prarit Bhargava
(Prarit: Second try at this one, not sure if this made it to LKML or not.
Sending to a wider audience this time)
Booting 2.6.32-rc5 on some IBM systems results in
Disabling IRQ #19
=================================
[ INFO: inconsistent lock state ]
2.6.32-rc5 #1
---------------------------------
inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
(&irq_desc_lock_class){?.-...}, at: [<ffffffff810c264e>] try_one_irq+0x32/0x138
{IN-HARDIRQ-W} state was registered at:
[<ffffffff81095160>] __lock_acquire+0x2fc/0xd5d
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c3389>] handle_level_irq+0x30/0x105
[<ffffffff81014e0e>] handle_irq+0x95/0xb7
[<ffffffff810141bd>] do_IRQ+0x6a/0xe0
[<ffffffff81012813>] ret_from_intr+0x0/0x16
irq event stamp: 195096
hardirqs last enabled at (195096): [<ffffffff814cd7f7>] _spin_unlock_irq+0x3a/0x5c
hardirqs last disabled at (195095): [<ffffffff814cdbdd>] _spin_lock_irq+0x29/0x95
softirqs last enabled at (195088): [<ffffffff81068c92>] __do_softirq+0x1c1/0x1ef
softirqs last disabled at (195093): [<ffffffff8101304c>] call_softirq+0x1c/0x30
other info that might help us debug this:
1 lock held by swapper/0:
#0: (kernel/irq/spurious.c:21){+.-...}, at: [<ffffffff81070cf2>] run_timer_softirq+0x1a9/0x315
stack backtrace:
Pid: 0, comm: swapper Not tainted 2.6.32-rc5 #1
Call Trace:
<IRQ> [<ffffffff81093e94>] valid_state+0x187/0x1ae
[<ffffffff81096c7b>] ? check_usage_backwards+0x0/0xa3
[<ffffffff81093fe4>] mark_lock+0x129/0x253
[<ffffffff810951d4>] __lock_acquire+0x370/0xd5d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff8109329d>] ? save_trace+0x4e/0xcd
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff810c264e>] try_one_irq+0x32/0x138
[<ffffffff810c2795>] poll_all_shared_irqs+0x41/0x6d
[<ffffffff810c27dd>] poll_spurious_irqs+0x1c/0x49
[<ffffffff81070d82>] run_timer_softirq+0x239/0x315
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c27c1>] ? poll_spurious_irqs+0x0/0x49
[<ffffffff81068bd3>] __do_softirq+0x102/0x1ef
[<ffffffff8108eccf>] ? tick_dev_program_event+0x46/0xcc
[<ffffffff8101304c>] call_softirq+0x1c/0x30
[<ffffffff81014b65>] do_softirq+0x59/0xca
[<ffffffff810686ad>] irq_exit+0x58/0xae
[<ffffffff81029b84>] smp_apic_timer_interrupt+0x94/0xba
[<ffffffff81012a33>] apic_timer_interrupt+0x13/0x20
<EOI> [<ffffffff8101a7b5>] ? mwait_idle+0x8c/0xb5
[<ffffffff8101a7ac>] ? mwait_idle+0x83/0xb5
[<ffffffff81010e55>] ? cpu_idle+0xbe/0x100
[<ffffffff814c4270>] ? start_secondary+0x219/0x270
This happens because the &desc->lock is taken with spin_lock_irqsave and
just a spin_lock. In the try_one_irq(), this lock really should be a
spin_lock_irqsave().
I have not yet narrowed down the reason for the spurious interrupt (although
I suspect it maybe to do with the radeon driver).
Successfully tested by me.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
--- linux-2.6.31.x86_64.orig/kernel/irq/spurious.c 2009-09-09 18:13:59.000000000 -0400
+++ linux-2.6.31.x86_64/kernel/irq/spurious.c 2009-10-26 10:55:56.709845786 -0400
@@ -27,8 +27,9 @@ static int try_one_irq(int irq, struct i
{
struct irqaction *action;
int ok = 0, work = 0;
+ unsigned long flags;
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
/* Already running on another processor */
if (desc->status & IRQ_INPROGRESS) {
/*
@@ -37,13 +38,13 @@ static int try_one_irq(int irq, struct i
*/
if (desc->action && (desc->action->flags & IRQF_SHARED))
desc->status |= IRQ_PENDING;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
return ok;
}
/* Honour the normal IRQ locking */
desc->status |= IRQ_INPROGRESS;
action = desc->action;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
while (action) {
/* Only shared IRQ handlers are safe to call */
@@ -56,7 +57,7 @@ static int try_one_irq(int irq, struct i
}
local_irq_disable();
/* Now clean up the flags */
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
action = desc->action;
/*
@@ -68,9 +69,9 @@ static int try_one_irq(int irq, struct i
* Perform real IRQ processing for the IRQ we deferred
*/
work = 1;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
handle_IRQ_event(irq, action);
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
desc->status &= ~IRQ_PENDING;
}
desc->status &= ~IRQ_INPROGRESS;
@@ -80,7 +81,7 @@ static int try_one_irq(int irq, struct i
*/
if (work && desc->chip && desc->chip->end)
desc->chip->end(irq);
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
return ok;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-03 13:45 [PATCH]: use spin_lock_irqsave in try_one_irq() Prarit Bhargava
@ 2009-11-03 14:58 ` Yong Zhang
2009-11-03 15:03 ` Prarit Bhargava
2009-11-04 9:18 ` Thomas Gleixner
0 siblings, 2 replies; 8+ messages in thread
From: Yong Zhang @ 2009-11-03 14:58 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel, ebiederm, akpm, Ingo Molnar, Thomas Gleixner
> This happens because the &desc->lock is taken with spin_lock_irqsave and
> just a spin_lock. In the try_one_irq(), this lock really should be a
> spin_lock_irqsave().
>
Cc'ed Ingo and Thomas.
The reason is that try_one_irq() is called both from hardirq context and softirq
context. And by default the timer handler poll_all_shared_irqs() is
called with irq enabled.
Then the two usage will cause inconsistent.
So I think the following patch is also workable to you.
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 114e704..11affbc 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -111,6 +111,7 @@ static void poll_all_shared_irqs(void)
for_each_irq_desc(i, desc) {
unsigned int status;
+ unsigned long flags;
if (!i)
continue;
@@ -121,7 +122,9 @@ static void poll_all_shared_irqs(void)
if (!(status & IRQ_SPURIOUS_DISABLED))
continue;
+ local_irq_save(flags);
try_one_irq(i, desc);
+ local_irq_restore(flags);
}
}
> I have not yet narrowed down the reason for the spurious interrupt (although
> I suspect it maybe to do with the radeon driver).
>
> Successfully tested by me.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>
> --- linux-2.6.31.x86_64.orig/kernel/irq/spurious.c 2009-09-09 18:13:59.000000000 -0400
> +++ linux-2.6.31.x86_64/kernel/irq/spurious.c 2009-10-26 10:55:56.709845786 -0400
> @@ -27,8 +27,9 @@ static int try_one_irq(int irq, struct i
> {
> struct irqaction *action;
> int ok = 0, work = 0;
> + unsigned long flags;
>
> - spin_lock(&desc->lock);
> + spin_lock_irqsave(&desc->lock, flags);
> /* Already running on another processor */
> if (desc->status & IRQ_INPROGRESS) {
> /*
> @@ -37,13 +38,13 @@ static int try_one_irq(int irq, struct i
> */
> if (desc->action && (desc->action->flags & IRQF_SHARED))
> desc->status |= IRQ_PENDING;
> - spin_unlock(&desc->lock);
> + spin_unlock_irqrestore(&desc->lock, flags);
> return ok;
> }
> /* Honour the normal IRQ locking */
> desc->status |= IRQ_INPROGRESS;
> action = desc->action;
> - spin_unlock(&desc->lock);
> + spin_unlock_irqrestore(&desc->lock, flags);
>
> while (action) {
> /* Only shared IRQ handlers are safe to call */
> @@ -56,7 +57,7 @@ static int try_one_irq(int irq, struct i
> }
> local_irq_disable();
> /* Now clean up the flags */
> - spin_lock(&desc->lock);
> + spin_lock_irqsave(&desc->lock, flags);
> action = desc->action;
>
> /*
> @@ -68,9 +69,9 @@ static int try_one_irq(int irq, struct i
> * Perform real IRQ processing for the IRQ we deferred
> */
> work = 1;
> - spin_unlock(&desc->lock);
> + spin_unlock_irqrestore(&desc->lock, flags);
> handle_IRQ_event(irq, action);
> - spin_lock(&desc->lock);
> + spin_lock_irqsave(&desc->lock, flags);
> desc->status &= ~IRQ_PENDING;
> }
> desc->status &= ~IRQ_INPROGRESS;
> @@ -80,7 +81,7 @@ static int try_one_irq(int irq, struct i
> */
> if (work && desc->chip && desc->chip->end)
> desc->chip->end(irq);
> - spin_unlock(&desc->lock);
> + spin_unlock_irqrestore(&desc->lock, flags);
>
> return ok;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-03 14:58 ` Yong Zhang
@ 2009-11-03 15:03 ` Prarit Bhargava
2009-11-04 9:18 ` Thomas Gleixner
1 sibling, 0 replies; 8+ messages in thread
From: Prarit Bhargava @ 2009-11-03 15:03 UTC (permalink / raw)
To: Yong Zhang; +Cc: linux-kernel, ebiederm, akpm, Ingo Molnar, Thomas Gleixner
Yong Zhang wrote:
>> This happens because the &desc->lock is taken with spin_lock_irqsave and
>> just a spin_lock. In the try_one_irq(), this lock really should be a
>> spin_lock_irqsave().
>>
>>
>
> Cc'ed Ingo and Thomas.
>
> The reason is that try_one_irq() is called both from hardirq context and softirq
> context. And by default the timer handler poll_all_shared_irqs() is
> called with irq enabled.
> Then the two usage will cause inconsistent.
>
> So I think the following patch is also workable to you.
>
Ah, okay. I will retest and get back to you ...
P.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-03 14:58 ` Yong Zhang
2009-11-03 15:03 ` Prarit Bhargava
@ 2009-11-04 9:18 ` Thomas Gleixner
2009-11-04 12:49 ` Yong Zhang
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2009-11-04 9:18 UTC (permalink / raw)
To: Yong Zhang; +Cc: Prarit Bhargava, linux-kernel, ebiederm, akpm, Ingo Molnar
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1174 bytes --]
On Tue, 3 Nov 2009, Yong Zhang wrote:
> > This happens because the &desc->lock is taken with spin_lock_irqsave and
> > just a spin_lock. In the try_one_irq(), this lock really should be a
> > spin_lock_irqsave().
> >
>
> Cc'ed Ingo and Thomas.
>
> The reason is that try_one_irq() is called both from hardirq context and softirq
> context. And by default the timer handler poll_all_shared_irqs() is
> called with irq enabled.
> Then the two usage will cause inconsistent.
>
> So I think the following patch is also workable to you.
Yes, that's sufficient.
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 114e704..11affbc 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -111,6 +111,7 @@ static void poll_all_shared_irqs(void)
>
> for_each_irq_desc(i, desc) {
> unsigned int status;
> + unsigned long flags;
>
> if (!i)
> continue;
> @@ -121,7 +122,9 @@ static void poll_all_shared_irqs(void)
> if (!(status & IRQ_SPURIOUS_DISABLED))
> continue;
>
> + local_irq_save(flags);
> try_one_irq(i, desc);
> + local_irq_restore(flags);
You can even use local_irq_en/disable() here.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-04 9:18 ` Thomas Gleixner
@ 2009-11-04 12:49 ` Yong Zhang
2009-11-04 13:01 ` Yong Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Yong Zhang @ 2009-11-04 12:49 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Prarit Bhargava, linux-kernel, ebiederm, akpm, Ingo Molnar
On Wed, Nov 4, 2009 at 5:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 3 Nov 2009, Yong Zhang wrote:
>
>> > This happens because the &desc->lock is taken with spin_lock_irqsave and
>> > just a spin_lock. In the try_one_irq(), this lock really should be a
>> > spin_lock_irqsave().
>> >
>> So I think the following patch is also workable to you.
>
> Yes, that's sufficient.
>
>> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
>> index 114e704..11affbc 100644
>> --- a/kernel/irq/spurious.c
>> +++ b/kernel/irq/spurious.c
>> @@ -111,6 +111,7 @@ static void poll_all_shared_irqs(void)
>>
>> for_each_irq_desc(i, desc) {
>> unsigned int status;
>> + unsigned long flags;
>>
>> if (!i)
>> continue;
>> @@ -121,7 +122,9 @@ static void poll_all_shared_irqs(void)
>> if (!(status & IRQ_SPURIOUS_DISABLED))
>> continue;
>>
>> + local_irq_save(flags);
>> try_one_irq(i, desc);
>> + local_irq_restore(flags);
>
> You can even use local_irq_en/disable() here.
Yup, I will resend the patch later.
Thanks,
Yong
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-04 12:49 ` Yong Zhang
@ 2009-11-04 13:01 ` Yong Zhang
2009-11-04 13:17 ` Prarit Bhargava
0 siblings, 1 reply; 8+ messages in thread
From: Yong Zhang @ 2009-11-04 13:01 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Prarit Bhargava, linux-kernel, ebiederm, akpm, Ingo Molnar
>From 83ee4c7591aae764f1656bd68f6e95ae112e2e7a Mon Sep 17 00:00:00 2001
From: Yong Zhang <yong.zhang0@gmail.com>
Date: Wed, 4 Nov 2009 20:52:45 +0800
Subject: [PATCH] genirq: try_one_irq() should be called with irq disabled
Prarit report this:
Booting 2.6.32-rc5 on some IBM systems results in
Disabling IRQ #19
=================================
[ INFO: inconsistent lock state ]
2.6.32-rc5 #1
---------------------------------
inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
(&irq_desc_lock_class){?.-...}, at: [<ffffffff810c264e>] try_one_irq+0x32/0x138
{IN-HARDIRQ-W} state was registered at:
[<ffffffff81095160>] __lock_acquire+0x2fc/0xd5d
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c3389>] handle_level_irq+0x30/0x105
[<ffffffff81014e0e>] handle_irq+0x95/0xb7
[<ffffffff810141bd>] do_IRQ+0x6a/0xe0
[<ffffffff81012813>] ret_from_intr+0x0/0x16
irq event stamp: 195096
hardirqs last enabled at (195096): [<ffffffff814cd7f7>]
_spin_unlock_irq+0x3a/0x5c
hardirqs last disabled at (195095): [<ffffffff814cdbdd>]
_spin_lock_irq+0x29/0x95
softirqs last enabled at (195088): [<ffffffff81068c92>]
__do_softirq+0x1c1/0x1ef
softirqs last disabled at (195093): [<ffffffff8101304c>] call_softirq+0x1c/0x30
other info that might help us debug this:
1 lock held by swapper/0:
#0: (kernel/irq/spurious.c:21){+.-...}, at: [<ffffffff81070cf2>]
run_timer_softirq+0x1a9/0x315
stack backtrace:
Pid: 0, comm: swapper Not tainted 2.6.32-rc5 #1
Call Trace:
<IRQ> [<ffffffff81093e94>] valid_state+0x187/0x1ae
[<ffffffff81096c7b>] ? check_usage_backwards+0x0/0xa3
[<ffffffff81093fe4>] mark_lock+0x129/0x253
[<ffffffff810951d4>] __lock_acquire+0x370/0xd5d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff8109329d>] ? save_trace+0x4e/0xcd
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff810c264e>] try_one_irq+0x32/0x138
[<ffffffff810c2795>] poll_all_shared_irqs+0x41/0x6d
[<ffffffff810c27dd>] poll_spurious_irqs+0x1c/0x49
[<ffffffff81070d82>] run_timer_softirq+0x239/0x315
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c27c1>] ? poll_spurious_irqs+0x0/0x49
[<ffffffff81068bd3>] __do_softirq+0x102/0x1ef
[<ffffffff8108eccf>] ? tick_dev_program_event+0x46/0xcc
[<ffffffff8101304c>] call_softirq+0x1c/0x30
[<ffffffff81014b65>] do_softirq+0x59/0xca
[<ffffffff810686ad>] irq_exit+0x58/0xae
[<ffffffff81029b84>] smp_apic_timer_interrupt+0x94/0xba
[<ffffffff81012a33>] apic_timer_interrupt+0x13/0x20
<EOI> [<ffffffff8101a7b5>] ? mwait_idle+0x8c/0xb5
[<ffffffff8101a7ac>] ? mwait_idle+0x83/0xb5
[<ffffffff81010e55>] ? cpu_idle+0xbe/0x100
[<ffffffff814c4270>] ? start_secondary+0x219/0x270
The reason is that try_one_irq() is called both from hardirq context
and softirq context. And by default the timer handler
poll_all_shared_irqs() is called with irq enabled.
Then the two usage will cause inconsistent.
Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
kernel/irq/spurious.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 114e704..bd7273e 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -121,7 +121,9 @@ static void poll_all_shared_irqs(void)
if (!(status & IRQ_SPURIOUS_DISABLED))
continue;
+ local_irq_disable();
try_one_irq(i, desc);
+ local_irq_enable();
}
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH]: use spin_lock_irqsave in try_one_irq()
2009-11-04 13:01 ` Yong Zhang
@ 2009-11-04 13:17 ` Prarit Bhargava
0 siblings, 0 replies; 8+ messages in thread
From: Prarit Bhargava @ 2009-11-04 13:17 UTC (permalink / raw)
To: Yong Zhang; +Cc: Thomas Gleixner, linux-kernel, ebiederm, akpm, Ingo Molnar
Yong Zhang wrote:
> From 83ee4c7591aae764f1656bd68f6e95ae112e2e7a Mon Sep 17 00:00:00 2001
> From: Yong Zhang <yong.zhang0@gmail.com>
> Date: Wed, 4 Nov 2009 20:52:45 +0800
> Subject: [PATCH] genirq: try_one_irq() should be called with irq disabled
>
> Prarit report this:
>
<snip>
Testing verifies that this patch works.
P.
> Reported-by: Prarit Bhargava <prarit@redhat.com>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> ---
> kernel/irq/spurious.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 114e704..bd7273e 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -121,7 +121,9 @@ static void poll_all_shared_irqs(void)
> if (!(status & IRQ_SPURIOUS_DISABLED))
> continue;
>
> + local_irq_disable();
> try_one_irq(i, desc);
> + local_irq_enable();
> }
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH]: use spin_lock_irqsave in try_one_irq()
@ 2009-10-26 18:35 Prarit Bhargava
0 siblings, 0 replies; 8+ messages in thread
From: Prarit Bhargava @ 2009-10-26 18:35 UTC (permalink / raw)
To: linux-kernel, ebiederm; +Cc: Prarit Bhargava
Booting 2.6.32-rc5 on some IBM systems results in
Disabling IRQ #19
=================================
[ INFO: inconsistent lock state ]
2.6.32-rc5 #1
---------------------------------
inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
(&irq_desc_lock_class){?.-...}, at: [<ffffffff810c264e>] try_one_irq+0x32/0x138
{IN-HARDIRQ-W} state was registered at:
[<ffffffff81095160>] __lock_acquire+0x2fc/0xd5d
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c3389>] handle_level_irq+0x30/0x105
[<ffffffff81014e0e>] handle_irq+0x95/0xb7
[<ffffffff810141bd>] do_IRQ+0x6a/0xe0
[<ffffffff81012813>] ret_from_intr+0x0/0x16
irq event stamp: 195096
hardirqs last enabled at (195096): [<ffffffff814cd7f7>] _spin_unlock_irq+0x3a/0x5c
hardirqs last disabled at (195095): [<ffffffff814cdbdd>] _spin_lock_irq+0x29/0x95
softirqs last enabled at (195088): [<ffffffff81068c92>] __do_softirq+0x1c1/0x1ef
softirqs last disabled at (195093): [<ffffffff8101304c>] call_softirq+0x1c/0x30
other info that might help us debug this:
1 lock held by swapper/0:
#0: (kernel/irq/spurious.c:21){+.-...}, at: [<ffffffff81070cf2>] run_timer_softirq+0x1a9/0x315
stack backtrace:
Pid: 0, comm: swapper Not tainted 2.6.32-rc5 #1
Call Trace:
<IRQ> [<ffffffff81093e94>] valid_state+0x187/0x1ae
[<ffffffff81096c7b>] ? check_usage_backwards+0x0/0xa3
[<ffffffff81093fe4>] mark_lock+0x129/0x253
[<ffffffff810951d4>] __lock_acquire+0x370/0xd5d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff8109329d>] ? save_trace+0x4e/0xcd
[<ffffffff81095cb4>] lock_acquire+0xf3/0x12d
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff814cdadd>] _spin_lock+0x40/0x89
[<ffffffff810c264e>] ? try_one_irq+0x32/0x138
[<ffffffff810c264e>] try_one_irq+0x32/0x138
[<ffffffff810c2795>] poll_all_shared_irqs+0x41/0x6d
[<ffffffff810c27dd>] poll_spurious_irqs+0x1c/0x49
[<ffffffff81070d82>] run_timer_softirq+0x239/0x315
[<ffffffff81070cf2>] ? run_timer_softirq+0x1a9/0x315
[<ffffffff810c27c1>] ? poll_spurious_irqs+0x0/0x49
[<ffffffff81068bd3>] __do_softirq+0x102/0x1ef
[<ffffffff8108eccf>] ? tick_dev_program_event+0x46/0xcc
[<ffffffff8101304c>] call_softirq+0x1c/0x30
[<ffffffff81014b65>] do_softirq+0x59/0xca
[<ffffffff810686ad>] irq_exit+0x58/0xae
[<ffffffff81029b84>] smp_apic_timer_interrupt+0x94/0xba
[<ffffffff81012a33>] apic_timer_interrupt+0x13/0x20
<EOI> [<ffffffff8101a7b5>] ? mwait_idle+0x8c/0xb5
[<ffffffff8101a7ac>] ? mwait_idle+0x83/0xb5
[<ffffffff81010e55>] ? cpu_idle+0xbe/0x100
[<ffffffff814c4270>] ? start_secondary+0x219/0x270
This happens because the &desc->lock is taken with spin_lock_irqsave and
just a spin_lock. In the try_one_irq(), this lock really should be a
spin_lock_irqsave().
I have not yet narrowed down the reason for the spurious interrupt (although
I suspect it maybe to do with the radeon driver).
Successfully tested by me.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
--- linux-2.6.31.x86_64.orig/kernel/irq/spurious.c 2009-09-09 18:13:59.000000000 -0400
+++ linux-2.6.31.x86_64/kernel/irq/spurious.c 2009-10-26 10:55:56.709845786 -0400
@@ -27,8 +27,9 @@ static int try_one_irq(int irq, struct i
{
struct irqaction *action;
int ok = 0, work = 0;
+ unsigned long flags;
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
/* Already running on another processor */
if (desc->status & IRQ_INPROGRESS) {
/*
@@ -37,13 +38,13 @@ static int try_one_irq(int irq, struct i
*/
if (desc->action && (desc->action->flags & IRQF_SHARED))
desc->status |= IRQ_PENDING;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
return ok;
}
/* Honour the normal IRQ locking */
desc->status |= IRQ_INPROGRESS;
action = desc->action;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
while (action) {
/* Only shared IRQ handlers are safe to call */
@@ -56,7 +57,7 @@ static int try_one_irq(int irq, struct i
}
local_irq_disable();
/* Now clean up the flags */
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
action = desc->action;
/*
@@ -68,9 +69,9 @@ static int try_one_irq(int irq, struct i
* Perform real IRQ processing for the IRQ we deferred
*/
work = 1;
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
handle_IRQ_event(irq, action);
- spin_lock(&desc->lock);
+ spin_lock_irqsave(&desc->lock, flags);
desc->status &= ~IRQ_PENDING;
}
desc->status &= ~IRQ_INPROGRESS;
@@ -80,7 +81,7 @@ static int try_one_irq(int irq, struct i
*/
if (work && desc->chip && desc->chip->end)
desc->chip->end(irq);
- spin_unlock(&desc->lock);
+ spin_unlock_irqrestore(&desc->lock, flags);
return ok;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-11-04 13:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-03 13:45 [PATCH]: use spin_lock_irqsave in try_one_irq() Prarit Bhargava
2009-11-03 14:58 ` Yong Zhang
2009-11-03 15:03 ` Prarit Bhargava
2009-11-04 9:18 ` Thomas Gleixner
2009-11-04 12:49 ` Yong Zhang
2009-11-04 13:01 ` Yong Zhang
2009-11-04 13:17 ` Prarit Bhargava
-- strict thread matches above, loose matches on Subject: below --
2009-10-26 18:35 Prarit Bhargava
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.