Hi Thomas and Edward,
This patch works fine for our problems, but I'm not sure if it works for the recent submit "genirq: fix regression in irqfixup, irqpoll" "52553ddffad76ccf192d4dd9ce88d5818f57f62a", which submitted by Edward Donovan.
Edward can you help verify it on your environment?
Thanks,
-Warner
-----Original Message-----
From: Thomas Gleixner [mailto:[email protected]]
Sent: 2012??11??23?? PM 5:09
To: Wang, Warner
Cc: Wang, Song-Bo (Stoney)
Subject: Re: need help on a DEADLOCK problem related to function try_one_irq()
On Thu, 22 Nov 2012, Thomas Gleixner wrote:
> Warner,
>
> On Thu, 22 Nov 2012, Wang, Warner wrote:
>
> please send such bug reports to the kernel mailinglist in the future.
>
> > We met a problem on some of our x86 server and after a few weeks
> > trace-down effort, we believe the problem is in the file
> > "linux/kernel/irq/spurious.c". We created a patch but we are not
> > 100% sure if it is correct or complete. That is why we want to
> > consult you.
>
> You spotted the problem right, but I'm not sure at the first glance,
> whether this is correct. I need to go back into history and figure out
> why we added that call in the first place. It looks fundamentally
> wrong.
>
> Thanks for analyzing it. I'll keep you posted on my findings.
Can you try the patch below ?
Thanks,
tglx
---
kernel/irq/spurious.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
Index: tip/kernel/irq/spurious.c
===================================================================
--- tip.orig/kernel/irq/spurious.c
+++ tip/kernel/irq/spurious.c
@@ -80,13 +80,11 @@ static int try_one_irq(int irq, struct i
/*
* All handlers must agree on IRQF_SHARED, so we test just the
- * first. Check for action->next as well.
+ * first.
*/
action = desc->action;
if (!action || !(action->flags & IRQF_SHARED) ||
- (action->flags & __IRQF_TIMER) ||
- (action->handler(irq, action->dev_id) == IRQ_HANDLED) ||
- !action->next)
+ (action->flags & __IRQF_TIMER))
goto out;
/* Already running on another processor */ @@ -104,7 +102,7 @@ static int try_one_irq(int irq, struct i
do {
if (handle_irq_event(desc) == IRQ_HANDLED)
ret = IRQ_HANDLED;
- action = desc->action;
+ action = action->next;
} while ((desc->istate & IRQS_PENDING) && action);
desc->istate &= ~IRQS_POLL_INPROGRESS;
out:
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Yes, I will try to do that in the next couple days. Thanks -
Ed
On Mon, Nov 26, 2012 at 3:09 AM, Wang, Warner <[email protected]> wrote:
> Hi Thomas and Edward,
>
> This patch works fine for our problems, but I'm not sure if it works for the recent submit "genirq: fix regression in irqfixup, irqpoll" "52553ddffad76ccf192d4dd9ce88d5818f57f62a", which submitted by Edward Donovan.
>
> Edward can you help verify it on your environment?
>
>
> Thanks,
> -Warner
>
> -----Original Message-----
> From: Thomas Gleixner [mailto:[email protected]]
> Sent: 2012年11月23日 PM 5:09
> To: Wang, Warner
> Cc: Wang, Song-Bo (Stoney)
> Subject: Re: need help on a DEADLOCK problem related to function try_one_irq()
>
> On Thu, 22 Nov 2012, Thomas Gleixner wrote:
>
>> Warner,
>>
>> On Thu, 22 Nov 2012, Wang, Warner wrote:
>>
>> please send such bug reports to the kernel mailinglist in the future.
>>
>> > We met a problem on some of our x86 server and after a few weeks
>> > trace-down effort, we believe the problem is in the file
>> > "linux/kernel/irq/spurious.c". We created a patch but we are not
>> > 100% sure if it is correct or complete. That is why we want to
>> > consult you.
>>
>> You spotted the problem right, but I'm not sure at the first glance,
>> whether this is correct. I need to go back into history and figure out
>> why we added that call in the first place. It looks fundamentally
>> wrong.
>>
>> Thanks for analyzing it. I'll keep you posted on my findings.
>
> Can you try the patch below ?
>
> Thanks,
>
> tglx
>
> ---
> kernel/irq/spurious.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> Index: tip/kernel/irq/spurious.c
> ===================================================================
> --- tip.orig/kernel/irq/spurious.c
> +++ tip/kernel/irq/spurious.c
> @@ -80,13 +80,11 @@ static int try_one_irq(int irq, struct i
>
> /*
> * All handlers must agree on IRQF_SHARED, so we test just the
> - * first. Check for action->next as well.
> + * first.
> */
> action = desc->action;
> if (!action || !(action->flags & IRQF_SHARED) ||
> - (action->flags & __IRQF_TIMER) ||
> - (action->handler(irq, action->dev_id) == IRQ_HANDLED) ||
> - !action->next)
> + (action->flags & __IRQF_TIMER))
> goto out;
>
> /* Already running on another processor */ @@ -104,7 +102,7 @@ static int try_one_irq(int irq, struct i
> do {
> if (handle_irq_event(desc) == IRQ_HANDLED)
> ret = IRQ_HANDLED;
> - action = desc->action;
> + action = action->next;
> } while ((desc->istate & IRQS_PENDING) && action);
> desc->istate &= ~IRQS_POLL_INPROGRESS;
> out: