2.6.20->2.6.21 - networking dies after random time

Quoted 14 lines

Hello, i have a very similar problem with 2.6.21 also;

2 3com NICs and they are failling randomly.

The kernel is a basic fedora 7 kernel (2.6.21-1.3228.fc7)
I found a bug report and added details here : https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243960

I'm not subcribed on this list, so please cc me if there is any questions.

JB

Quoted 3 lines · Marcin Ślusarz

...

Quoted 1 line

I reproduced it on minimal config:

...

Quoted 1 line

Quoted 1 line

We know your hardware should be OK - since it was fine with 2.6.20.

...

It looks like there is something common in the air...

Marcin: ne2k_pci with 8390, Jean: 3com, and now I see similar problem with 8139cp too (plus some ideas):

http://marc.info/?l=linux-netdev&m=118293314109648&w=2

So, you probably should wait a little & look for new patches here.

Cheers,
Jarek P.

Update...
I did 2 tests :

1) booted with option acpi=off
It booted correctly, i managed to get some load on one of the card and after a while (10 minutes i guess) the Timeout occurs. Side effect, at the same moment the sata contolers lost control of the disks somehow and the raid 5 array on the system crashed hard. I have no traces as i was unable to rebuild it (and i tried a lot of extreme voodoo methods).

2) changed the 3com cards
i replaced by two cards,
01:06.0 Ethernet controller: VIA Technologies, Inc. VT6102 [Rhine-II] (rev 42) 01:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8029(AS)

reinstalled and stressed the network (small download from a laptop) and :

Jun 29 09:34:10 loki kernel: NETDEV WATCHDOG: eth0: transmit timed out
Jun 29 09:34:51 loki last message repeated 14 times
Jun 29 09:35:18 loki last message repeated 8 times

so it seems to be a more generic problem.

Ok, I've bisected this problem and found that this patch broke my NIC:

76d2160147f43f982dfe881404cfde9fd0a9da21 is first bad commit commit 76d2160147f43f982dfe881404cfde9fd0a9da21

Author: Ingo Molnar <[email protected]>
Date:   Fri Feb 16 01:28:24 2007 -0800

[PATCH] genirq: do not mask interrupts by default

So I cooked patch like below and everything is working fine (so far)

Fix default_disable interrupt function (broken by [PATCH] genirq: do not mask interrupts by default) - revert removal of codepath which was invoked when removed flag (IRQ_DELAYED_DISABLE) wag NOT set

Signed-off-by Marcin Slusarz <[email protected]>

---

kernel/irq/chip.c +0 -0

index 76a9106..0bb23cd 100644

Ok, I've bisected this problem and found that this patch broke my NIC:

76d2160147f43f982dfe881404cfde9fd0a9da21 is first bad commit commit 76d2160147f43f982dfe881404cfde9fd0a9da21

Author: Ingo Molnar <[email protected]>
Date:   Fri Feb 16 01:28:24 2007 -0800

[PATCH] genirq: do not mask interrupts by default

Never mask interrupts immediately upon request. Disabling interrupts in high-performance codepaths is rare, and on the other hand this change could
recover lost edges (or even other types of lost interrupts) by
conservatively
only masking interrupts after they happen. (NOTE: with this change the highlevel irq-disable code still soft-disables this IRQ line - and
if such an
interrupt happens then the IRQ flow handler keeps the IRQ masked.)

Mark i8529A controllers as 'never loses an edge'.

Signed-off-by Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

2) changed the 3com cards
i replaced by two cards,
01:06.0 Ethernet controller: VIA Technologies, Inc. VT6102 [Rhine-II] (rev 42) 01:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8029(AS)

reinstalled and stressed the network (small download from a laptop) and :

Jun 29 09:34:10 loki kernel: NETDEV WATCHDOG: eth0: transmit timed out
Jun 29 09:34:51 loki last message repeated 14 times
Jun 29 09:35:18 loki last message repeated 8 times

From include/linux/irq.h:
/**
* struct irq_chip - hardware interrupt chip descriptor

* @disable: disable the interrupt (defaults to chip->mask if NULL)

Ok, I've bisected this problem and found that this patch broke my NIC:

76d2160147f43f982dfe881404cfde9fd0a9da21 is first bad commit commit 76d2160147f43f982dfe881404cfde9fd0a9da21

Author: Ingo Molnar <[email protected]>
Date:   Fri Feb 16 01:28:24 2007 -0800

[PATCH] genirq: do not mask interrupts by default

thanks for tracking it down! Could you try the patch below (ontop an otherwise unmodified kernel)? This tests the theory whether the problem is related to the disable_irq_nosync() call in the ne2k driver's xmit path. Does this solve the hangs too?

please try the patch below instead.

Quoted 2 lines · Ingo Molnar

please try the patch below instead.

I'm hoping this is just a "let's see if the behavior changes" patch, not something that you think should be applied if it fixes something?

This patch looks like it is trying to paper over (rather than fix) some possible bug in the "->disable" logic. Makes sense as a "let's see if it's that" kind of thing, but not as a "let's fix it".

Or am I missing something?

Marcin, could you try the patch below too? [without having any other patch applied.] It basically turns the critical section into an irqs-off critical section and thus checks whether your problem is related to that particular area of code.

Quoted 5 lines · Ingo Molnar

Marcin, could you try the patch below too? [without having any other patch applied.] It basically turns the critical section into an irqs-off critical section and thus checks whether your problem is related to that particular area of code.

I read back on this thread and I think the problem is somewhere else:

static void ack_ioapic_quirk_irq(unsigned int irq)
...
/*
* It appears there is an erratum which affects at least version 0x11
* of I/O APIC (that's the 82093AA and cores integrated into various

* chipsets). Under certain conditions a level-triggered interrupt is
* erroneously delivered as edge-triggered one but the respective IRR
* bit gets set nevertheless. As a result the I/O unit expects an EOI
* message but it will never arrive and further interrupts are blocked
* from the source. The exact reason is so far unknown, but the
* phenomenon was observed when two consecutive interrupt requests
* from a given source get delivered to the same CPU and the source is
* temporarily disabled in between.

static void setup_IO_APIC_irq(int apic, int pin, unsigned int irq,
int trigger, int polarity)

       /* Mask level triggered irqs.
        * Use IRQ_DELAYED_DISABLE for edge triggered irqs.
        */

Looking into the IO_APIC code, the resend via send_IPI_self() happens unconditionally. So the resend is done for level and edge interrupts. This makes the problem more mysterious.

The code in question lib8390.c does

	disable_irq();
	fiddle_with_the_network_card_hardware()
	enable_irq();

No idea how this affects the network card, as the code there must be able to handle interrupts, which are not originated from the card due to interrupt sharing.

Quoted 5 lines

The code in question lib8390.c does

	disable_irq();
	fiddle_with_the_network_card_hardware()
	enable_irq();

...

Quoted 4 lines

No idea how this affects the network card, as the code there must be able to handle interrupts, which are not originated from the card due to interrupt sharing.

I think, in this last yesterday's patch Ingo could be right, yet! The comment at the beginnig points this is done like that because of chip's slowness. And problems with timing are mysterious.

On the other hand author of this code didn't use spin_lock_irqsave for some reason, probably after testing this option too. So, I hope this is the right path, but alas, I'm not sure this patch has to prove this 100%.

Anyway, in my opinion this situation where interrupts could/have_to be used for such strange things should confirm the need of more options for handling irqs individually.

Quoted 11 lines

Quoted 5 lines

The code in question lib8390.c does

	disable_irq();
	fiddle_with_the_network_card_hardware()
	enable_irq();

...

Quoted 4 lines

No idea how this affects the network card, as the code there must be able to handle interrupts, which are not originated from the card due to interrupt sharing.

Quoted 8 lines

I think, in this last yesterday's patch Ingo could be right, yet! The comment at the beginnig points this is done like that because of chip's slowness. And problems with timing are mysterious.

On the other hand author of this code didn't use spin_lock_irqsave for some reason, probably after testing this option too. So, I hope this is the right path, but alas, I'm not sure this patch has to prove this 100%.

The author (me) didn't use spin_lock_irqsave because the slowness of the card means that approach caused horrible problems like losing serial data at 38400 baud on some chips. Rememeber many 8390 nics on PCI were ISA chips with FPGA front ends.

Quoted 3 lines

Anyway, in my opinion this situation where interrupts could/have_to be used for such strange things should confirm the need of more options for handling irqs individually.

Ok the logic behind the 8390 is very simple:

Things to know
- IRQ delivery is asynchronous to the PCI bus
- Blocking the local CPU IRQ via spin locks was too slow
- The chip has register windows needing locking work

So the path was once (I say once as people appear to have changed it in the mean time and it now looks rather bogus if the changes to use disable_irq_nosync_irqsave are disabling the local IRQ)

Take the page lock
Mask the IRQ on chip
Disable the IRQ (but not mask locally- someone seems to have
broken this with the lock validator stuff) [This must be _nosync as the page lock may otherwise
deadlock us]
Drop the page lock and turn IRQs back on

At this point an existing IRQ may still be running but we can't get a new one

Take the lock (so we know the IRQ has terminated) but don't mask
the IRQs on the processor
Set irqlock [for debug]

Transmit (slow as ****)

re-enable the IRQ

We have to use disable_irq because otherwise you will get delayed interrupts on the APIC bus deadlocking the transmit path.

Quite hairy but the chip simply wasn't designed for SMP and you can't even ACK an interrupt without risking corrupting other parallel activities on the chip.

Alan

Hi,

Very below is my patch proposal with a comment, which in my opinion is precious enough to save it for future help in reading and understanding the code.

I hope Alan will not blame me I've not asked for his permission before sending, and he would ack this patch as it is or at least most of this.

Hi,

Very below is my patch proposal with a comment, which in my opinion is precious enough to save it for future help in reading and understanding the code.

I hope Alan will not blame me I've not asked for his permission before sending, and he would ack this patch as it is or at least most of this.

Thanks & regards,
Jarek P.

Quoted 57 lines · Alan Cox

Quoted 9 lines

Quoted 5 lines

The code in question lib8390.c does

	disable_irq();
	fiddle_with_the_network_card_hardware()
	enable_irq();

...

Quoted 3 lines

No idea how this affects the network card, as the code there must be able to handle interrupts, which are not originated from the card due to interrupt sharing.

Quoted 8 lines

I think, in this last yesterday's patch Ingo could be right, yet! The comment at the beginnig points this is done like that because of chip's slowness. And problems with timing are mysterious.

On the other hand author of this code didn't use spin_lock_irqsave for some reason, probably after testing this option too. So, I hope this is the right path, but alas, I'm not sure this patch has to prove this 100%.

The author (me) didn't use spin_lock_irqsave because the slowness of the card means that approach caused horrible problems like losing serial data at 38400 baud on some chips. Rememeber many 8390 nics on PCI were ISA chips with FPGA front ends.

Quoted 3 lines

Anyway, in my opinion this situation where interrupts could/have_to be used for such strange things should confirm the need of more options for handling irqs individually.

Ok the logic behind the 8390 is very simple:

Things to know
- IRQ delivery is asynchronous to the PCI bus
- Blocking the local CPU IRQ via spin locks was too slow
- The chip has register windows needing locking work

So the path was once (I say once as people appear to have changed it in the mean time and it now looks rather bogus if the changes to use disable_irq_nosync_irqsave are disabling the local IRQ)

Take the page lock
Mask the IRQ on chip
Disable the IRQ (but not mask locally- someone seems to have
broken this with the lock validator stuff) [This must be _nosync as the page lock may otherwise
deadlock us]
Drop the page lock and turn IRQs back on

At this point an existing IRQ may still be running but we can't get a new one

Take the lock (so we know the IRQ has terminated) but don't mask
the IRQs on the processor
Set irqlock [for debug]

Transmit (slow as ****)

re-enable the IRQ

Quoted 9 lines

We have to use disable_irq because otherwise you will get delayed interrupts on the APIC bus deadlocking the transmit path.

Quite hairy but the chip simply wasn't designed for SMP and you can't even ACK an interrupt without risking corrupting other parallel activities on the chip.

Alan

------>

From: Jarek Poplawski <[email protected]>

Subject: lib8390: comment on locking by Alan Cox

Additional explanation of problems with locking by Alan Cox.

Signed-off-by Jarek Poplawski <[email protected]>

Cc Alan Cox <[email protected]>

Cc Paul Gortmaker <[email protected]>

Cc Jeff Garzik <[email protected]>

Ok the logic behind the 8390 is very simple:

Things to know
- IRQ delivery is asynchronous to the PCI bus
- Blocking the local CPU IRQ via spin locks was too slow
- The chip has register windows needing locking work

So the path was once (I say once as people appear to have changed it in the mean time and it now looks rather bogus if the changes to use disable_irq_nosync_irqsave are disabling the local IRQ)

Take the page lock
Mask the IRQ on chip
Disable the IRQ (but not mask locally- someone seems to have
broken this with the lock validator stuff) [This must be _nosync as the page lock may otherwise
deadlock us]

Drop the page lock and turn IRQs back on

At this point an existing IRQ may still be running but we can't get a new one

Take the lock (so we know the IRQ has terminated) but don't mask
the IRQs on the processor
Set irqlock [for debug]

Transmit (slow as ****)

re-enable the IRQ

We have to use disable_irq because otherwise you will get delayed interrupts on the APIC bus deadlocking the transmit path.

Quite hairy but the chip simply wasn't designed for SMP and you can't even ACK an interrupt without risking corrupting other parallel activities on the chip.

So the whole locking is to be able to keep irqs enabled for a long time, without risking entry of the same IRQ handler on this same CPU, correct?

So it seems to me that maybe the driver could be surprised via these spurious interrupts that happen right after the irq_enable(). Does the patch below make any sense in your opinion?

(...)

2007/7/25, Thomas Gleixner <[email protected]>:

Quoted 1 line

(...)

I've tested Jarek's patch, 2 Ingo's patches (2nd and 3rd) and Thomas' patch (one patch at time of course) - all of them fixed the problem, but the last one flooded my logs with "Skip resend for irq 17". All tests were done on 2.6.21.3.

I wanted to test them all on 2.6.22.1, but I didn't have enough time. I've verified only that 2.6.22.1 has the same problem. I can test it later, but I can report results back at beginning of next week.

So, everything is clear - any changes are good!
Except the signed-off ones...

Quoted 3 lines

I wanted to test them all on 2.6.22.1, but I didn't have enough time. I've verified only that 2.6.22.1 has the same problem. I can test it later, but I can report results back at beginning of next week.

So, everything is clear - any changes are good!
Except the signed-off ones...

Thanks Marcin,
Jarek P.

PS: Now, it seems to me Thomas could be the nearest. BTW, could somebody give me some tip, how these re-triggered interrupts are skipped on dev's reset before enable_irq?

The other question is:

Is the driver confused by the resent irq or is the chip-set unhappy about the resend ?

We could figure the latter out by activating the software based resend method.

yeah. The patch below enables sw-resend on x86, to test the theory whether the APIC-driven hardware-vector-resend code has some problem.

On Thu, Jul 26, 2007 at 10:31:20AM +0200, Ingo Molnar wrote:
...

Quoted 2 lines

yeah. The patch below enables sw-resend on x86, to test the theory whether the APIC-driven hardware-vector-resend code has some problem.

I think Marcin is using x86_64 (Athlon 64) yet.

(..)
yeah - i meant to cover both arches but forgot about x86_64 - updated patch attached below.

Ingo

----------------->
Subject: x86: activate HARDIRQS_SW_RESEND
From: Ingo Molnar <[email protected]>

activate the software-triggered IRQ-resend logic.

it appears some chipsets/cpus do not handle local-APIC driven IRQ resends all that well, so always use the soft mechanism to trigger the execution of pending interrupts.

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/i386/Kconfig   |    4 ++++
 arch/x86_64/Kconfig |    4 ++++
 kernel/irq/manage.c |    8 ++++++++
 3 files changed, 16 insertions(+)

Index: linux-rt-rebase.q/arch/i386/Kconfig
===================================================================

arch/i386/Kconfig +4 -0

@@ -1284,6 +1284,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 config X86_SMP
        bool
        depends on SMP && !X86_VOYAGER

Index: linux-rt-rebase.q/arch/x86_64/Kconfig
===================================================================

arch/x86_64/Kconfig +4 -0

@@ -721,6 +721,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 menu "Power management options"
 
 source kernel/power/Kconfig

Index: linux-rt-rebase.q/kernel/irq/manage.c
===================================================================

kernel/irq/manage.c +0 -0

@@ -175,6 +175,14 @@ void enable_irq(unsigned int irq)
                desc->depth--;
        }
        spin_unlock_irqrestore(&desc->lock, flags);

Quoted 4 lines

Subject: x86: activate HARDIRQS_SW_RESEND
From: Ingo Molnar <[email protected]>

activate the software-triggered IRQ-resend logic.

This patch didn't help (tested on 2.6.22.1) - ne2k_pci timed out.

(..)
does the patch below fix those timeouts? It tests the theory whether any POST latency could expose this problem.

Ingo

Index: linux/drivers/net/lib8390.c
===================================================================

Review

Quoted 7 lines

        ei_outb_p(ENISR_ALL, e8390_base + EN0_IMR);

(quoted diff) +2 -0

+       /* force POST: */
+       ei_inb_p(e8390_base + EN0_IMR);
 
        spin_unlock(&ei_local->page_lock);
        enable_irq_lockdep_irqrestore(dev->irq, &flags);

Bad news. It doesn't fix the problem.

ok, it wasnt supposed to be _that_ easy i guess :-) Can you please (re-)confirm that the workaround below indeed fixes the hung card problem? (after producing a single WARN_ON message into the syslog)

also, does removing the ne2k-pci module and reinserting it again solve the problem too, or is your network card stuck forever once it got into that state?

ps: I retested all patches posted in this thread on top of 2.6.22.1 and behavior from 2.6.21.3 didn't changed. My next tests will be on 2.6.22.x only.

Marcin,

I see you're quite busy, but if after testing this next Ingo's patch you are alive yet, maybe you could try one more "idea"? No patch this time, but if you could try this after adding boot option "noirqdebug" (I'd like to be sure it's not about timinig after all).

2007/7/31, Jarek Poplawski <[email protected]>:

Quoted 6 lines

Marcin,

I see you're quite busy, but if after testing this next Ingo's patch you are alive yet, maybe you could try one more "idea"? No patch this time, but if you could try this after adding boot option "noirqdebug" (I'd like to be sure it's not about timinig after all).

It didn't change anything. Network card still timed out.

But, since level types don't need this retriggers too much I think this "don't mask interrupts by default" idea should be rethinked:
is there enough gain to risk such hard to diagnose errors?

Before, they would print:

eth0: transmit timed out, tx_status 00 status e601.
diagnostics: net 0ccc media 8880 dma 0000003a fifo 0000
eth0: Interrupt posted but not delivered -- IRQ blocked by another device?
Flags; bus-master 1, dirty 295757(13) current 295757(13) Transmit list 00000000 vs. f7150a20.

  0: @f7150200  length 80000070 status 0c010070
  1: @f71502a0  length 80000070 status 0c010070
  2: @f7150340  length 8000005c status 0c01005c

Now they just work, apparently...

Quoted 6 lines · Jarek Poplawski

It seems, we can start to think about some preferred solutions, already. Here are some of my preliminary conclusions and suggestions.

The problem of timeouts with some 'older' network cards seems to hit mainly x86_64 arch, and after diagnosing and testing (still beeing done) it's caused by resending level type irqs.

i386 interrupt code should be similar, except for the lack of per CPU irqs, but that shouldnt' affect resending.

Quoted 1 line

Quoted 1 line

Possible solutions:

We should probably at least add some statistic counters to the standard kernel to try to detect these cases.

Quoted 1 line

It looks like these changes are needed for this x86_64 only,

Why?

Quoted 6 lines · Ingo Molnar

But, since level types don't need this retriggers too much I think this "don't mask interrupts by default" idea should be rethinked:
is there enough gain to risk such hard to diagnose errors?

I reverted those masking changes in Fedora and the baffling problem with 3Com 3C905 network adapters went away.

Before, they would print:

eth0: transmit timed out, tx_status 00 status e601.
diagnostics: net 0ccc media 8880 dma 0000003a fifo 0000
eth0: Interrupt posted but not delivered -- IRQ blocked by another device?
Flags; bus-master 1, dirty 295757(13) current 295757(13) Transmit list 00000000 vs. f7150a20.

  0: @f7150200  length 80000070 status 0c010070
  1: @f71502a0  length 80000070 status 0c010070
  2: @f7150340  length 8000005c status 0c01005c

Now they just work, apparently...

So why not just revert the change?

(..)
please try Jarek's second patch too - there was a missing unmask.

Ingo

-------------->
Subject: genirq: fix simple and fasteoi irq handlers
From: Jarek Poplawski <[email protected]>

After the "genirq: do not mask interrupts by default" patch interrupts should be disabled not immediately upon request, but after they happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a driver's work.

The main reason of problems here, pointing the broken patch and making the first patch which can fix this was done by Marcin Slusarz. Additional test patches of Thomas Gleixner and Ingo Molnar tested by Marcin Slusarz helped to narrow possible reasons even more. Thanks.

PS: this patch fixes only one evident error here, but there could be more places affected by above-mentioned change in irq handling.

PS 2:
After rethinking, IMHO, there are two most probable scenarios here:

1. After hw resend there could be a conflict between retriggered edge type irq and the next level type one: e.g. if this level type irq (io_apic is enabled then) is triggered while retriggered irq is serviced (IRQ_INPROGRESS) there is goto out with eoi, and probably the next such levels are triggered and looping, so probably kind of flood in io_apic until this retriggered edge service has ended.
2. There is something wrong with ioapic_retrigger_irq (less probable because this should be probably seen with 'normal' edge retriggers, but on the other hand, they could be less common).

So, if there is #1, this fixed patch should work.

But, since level types don't need this retriggers too much I think this "don't mask interrupts by default" idea should be rethinked:
is there enough gain to risk such hard to diagnose errors?

So, IMHO, there should be at least possibility to turn this off for level types in config (it should be a visible option, so people could find & try this before writing for help or changing a network card).

Signed-off-by Jarek Poplawski <[email protected]>

---

diff -Nurp 2.6.23-rc1-/kernel/irq/chip.c 2.6.23-rc1/kernel/irq/chip.c

kernel/irq/chip.c 2007-08-05 21:49:46.000000000 +0200 renamed from kernel/irq/chip.c 2007-07-09 01:32:17.000000000 +0200 +7 -8

@@ -295,12 +295,11 @@ handle_simple_irq(unsigned int irq, stru
 
        spin_lock(&desc->lock);
 
-       if (unlikely(desc->status & IRQ_INPROGRESS))
-               goto out_unlock;
        kstat_cpu(cpu).irqs[irq]++;
 
        action = desc->action;
-       if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+       if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
+                                                IRQ_DISABLED)))) {
                if (desc->chip->mask)
                        desc->chip->mask(irq);
                desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);

@@ -318,6 +317,8 @@ handle_simple_irq(unsigned int irq, stru
 
        spin_lock(&desc->lock);
        desc->status &= ~IRQ_INPROGRESS;
+       if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
+               desc->chip->unmask(irq);
 out_unlock:
        spin_unlock(&desc->lock);
 }

@@ -392,18 +393,16 @@ handle_fasteoi_irq(unsigned int irq, str
 
        spin_lock(&desc->lock);
 
-       if (unlikely(desc->status & IRQ_INPROGRESS))
-               goto out;
-
        desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
        kstat_cpu(cpu).irqs[irq]++;
 
        /*
-        * If its disabled or no action available
+        * If it's running, disabled or no action available
         * then mask it and get out of here:
         */
        action = desc->action;
-       if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+       if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
+                                                IRQ_DISABLED)))) {
                desc->status |= IRQ_PENDING;
                if (desc->chip->mask)
                        desc->chip->mask(irq);

2007/8/6, Ingo Molnar <[email protected]>:

Quoted 8 lines

(..)
please try Jarek's second patch too - there was a missing unmask.

Ingo

-------------->
Subject: genirq: fix simple and fasteoi irq handlers
From: Jarek Poplawski <[email protected]>

Network card still locks up (tested on 2.6.22.1). I had to upload more data than usual (~350 MB vs ~1-100 MB) to trigger that bug but it might be a coincidence...

2007/8/7, Jarek Poplawski <[email protected]>:

Quoted 9 lines · Marcin Ślusarz

Quoted 3 lines

Network card still locks up (tested on 2.6.22.1). I had to upload more data than usual (~350 MB vs ~1-100 MB) to trigger that bug but it might be a coincidence...

Thanks! It's a good news after all - it would be really strange why this place doesn't hit more people (it seems there is some safety elsewhere for this).

Quoted 2 lines

BTW: I hope, this previous Thomas' patch with Ingo's warning to resend.c (with a warning), had no problems with a similar load?

I always tested on 500-600 MB "dataset"

Quoted 1 line

PS: Marcin, if you need a break in this testing let us know!

No, i don't need a break. I'll have more time in next weeks.

Quoted 12 lines · Marcin Ślusarz

2007/8/7, Jarek Poplawski <[email protected]>:

Quoted 11 lines · Marcin Ślusarz

Quoted 3 lines

Network card still locks up (tested on 2.6.22.1). I had to upload more data than usual (~350 MB vs ~1-100 MB) to trigger that bug but it might be a coincidence...

Thanks! It's a good news after all - it would be really strange why this place doesn't hit more people (it seems there is some safety elsewhere for this).

BTW: I hope, this previous Thomas' patch with Ingo's warning to resend.c (with a warning), had no problems with a similar load?

Quoted 4 lines

I always tested on 500-600 MB "dataset"

Quoted 1 line

PS: Marcin, if you need a break in this testing let us know!

No, i don't need a break. I'll have more time in next weeks.

Great! So, I'll try to send a patch with _SW_RESEND in a few hours, if Ingo doesn't prepare something for you.

Quoted 1 line · Jarek Poplawski

On Tue, Aug 07, 2007 at 11:37:01AM +0200, Marcin Ślusarz wrote:

Quoted 4 lines

Quoted 1 line

No, i don't need a break. I'll have more time in next weeks.

Great! So, I'll try to send a patch with _SW_RESEND in a few hours, if Ingo doesn't prepare something for you.

So, the let's try this idea yet: modified Ingo's "x86: activate HARDIRQS_SW_RESEND" patch.
(Don't forget about make oldconfig before make.)
For testing only.

Cheers,
Jarek P.

PS: alas there was not even time for "compile checking"...

And here is one more patch to test the same idea (chip->retrigger()). Let's try i386 way! (I hope I will not be arrested for this...) (Should be tested without any previous patches.)

Jarek P.

PS: as above

---

diff -Nurp 2.6.22.1-/arch/x86_64/kernel/io_apic.c 2.6.22.1/arch/x86_64/kernel/io_apic.c

arch/x86_64/kernel/io_apic.c 2007-08-07 14:37:45.000000000 +0200 renamed from arch/x86_64/kernel/io_apic.c 2007-07-09 01:32:17.000000000 +0200 +0 -3

@@ -1311,15 +1311,8 @@ static unsigned int startup_ioapic_irq(u
 static int ioapic_retrigger_irq(unsigned int irq)
 {
        struct irq_cfg *cfg = &irq_cfg[irq];
-       cpumask_t mask;
-       unsigned long flags;
-

So, the let's try this idea yet: modified Ingo's "x86: activate HARDIRQS_SW_RESEND" patch.
(Don't forget about make oldconfig before make.)
For testing only.

Cheers,
Jarek P.

PS: alas there was not even time for "compile checking"...

---

diff -Nurp 2.6.22.1-/arch/i386/Kconfig 2.6.22.1/arch/i386/Kconfig

arch/i386/Kconfig 2007-08-07 13:13:03.000000000 +0200 renamed from arch/i386/Kconfig 2007-07-09 01:32:17.000000000 +0200 +4 -0

@@ -1252,6 +1252,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 config X86_SMP
        bool
        depends on SMP && !X86_VOYAGER

diff -Nurp 2.6.22.1-/arch/x86_64/Kconfig 2.6.22.1/arch/x86_64/Kconfig

arch/x86_64/Kconfig 2007-08-07 13:13:03.000000000 +0200 renamed from arch/x86_64/Kconfig 2007-07-09 01:32:17.000000000 +0200 +4 -0

@@ -690,6 +690,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 menu "Power management options"
 
 source kernel/power/Kconfig

diff -Nurp 2.6.22.1-/kernel/irq/manage.c 2.6.22.1/kernel/irq/manage.c

kernel/irq/manage.c 2007-08-07 13:13:03.000000000 +0200 renamed from kernel/irq/manage.c 2007-07-09 01:32:17.000000000 +0200 +8 -0

@@ -169,6 +169,14 @@ void enable_irq(unsigned int irq)
                desc->depth--;
        }
        spin_unlock_irqrestore(&desc->lock, flags);
+#ifdef CONFIG_HARDIRQS_SW_RESEND
+       /*
+        * Do a bh disable/enable pair to trigger any pending
+        * irq resend logic:
+        */
+       local_bh_disable();
+       local_bh_enable();
+#endif
 }
 EXPORT_SYMBOL(enable_irq);
 

diff -Nurp 2.6.22.1-/kernel/irq/resend.c 2.6.22.1/kernel/irq/resend.c

kernel/irq/resend.c 2007-08-07 13:57:54.000000000 +0200 renamed from kernel/irq/resend.c 2007-07-09 01:32:17.000000000 +0200 +10 -3

@@ -62,16 +62,24 @@ void check_irq_resend(struct irq_desc *d
         */
        desc->chip->enable(irq);
 
+       /*
+        * Temporary hack to figure out more about the problem, which
+        * is causing the ancient network cards to die.
+        */
+
        if ((status & (IRQ_PENDING | IRQ_REPLAY)) == IRQ_PENDING) {
                desc->status = (status & ~IRQ_PENDING) | IRQ_REPLAY;
 
-               if (!desc->chip || !desc->chip->retrigger ||
-                                       !desc->chip->retrigger(irq)) {
+               if (desc->handle_irq == handle_edge_irq) {
+                       if (desc->chip->retrigger)
+                               desc->chip->retrigger(irq);
+                       return;
+               }
 #ifdef CONFIG_HARDIRQS_SW_RESEND
-                       /* Set it pending and activate the softirq: */

2007/8/7, Jarek Poplawski <[email protected]>:

Quoted 86 lines

So, the let's try this idea yet: modified Ingo's "x86: activate HARDIRQS_SW_RESEND" patch.
(Don't forget about make oldconfig before make.)
For testing only.

Cheers,
Jarek P.

PS: alas there was not even time for "compile checking"...

---

diff -Nurp 2.6.22.1-/arch/i386/Kconfig 2.6.22.1/arch/i386/Kconfig

arch/i386/Kconfig 2007-08-07 13:13:03.000000000 +0200 renamed from arch/i386/Kconfig 2007-07-09 01:32:17.000000000 +0200 +4 -0

@@ -1252,6 +1252,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 config X86_SMP
        bool
        depends on SMP && !X86_VOYAGER

diff -Nurp 2.6.22.1-/arch/x86_64/Kconfig 2.6.22.1/arch/x86_64/Kconfig

arch/x86_64/Kconfig 2007-08-07 13:13:03.000000000 +0200 renamed from arch/x86_64/Kconfig 2007-07-09 01:32:17.000000000 +0200 +4 -0

@@ -690,6 +690,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND
+       bool
+       default y
+
 menu "Power management options"
 
 source kernel/power/Kconfig

diff -Nurp 2.6.22.1-/kernel/irq/manage.c 2.6.22.1/kernel/irq/manage.c

kernel/irq/manage.c 2007-08-07 13:13:03.000000000 +0200 renamed from kernel/irq/manage.c 2007-07-09 01:32:17.000000000 +0200 +8 -0

@@ -169,6 +169,14 @@ void enable_irq(unsigned int irq)
                desc->depth--;
        }
        spin_unlock_irqrestore(&desc->lock, flags);
+#ifdef CONFIG_HARDIRQS_SW_RESEND
+       /*
+        * Do a bh disable/enable pair to trigger any pending
+        * irq resend logic:
+        */
+       local_bh_disable();
+       local_bh_enable();
+#endif
 }
 EXPORT_SYMBOL(enable_irq);
 

diff -Nurp 2.6.22.1-/kernel/irq/resend.c 2.6.22.1/kernel/irq/resend.c

kernel/irq/resend.c 2007-08-07 13:57:54.000000000 +0200 renamed from kernel/irq/resend.c 2007-07-09 01:32:17.000000000 +0200 +12 -5

@@ -62,16 +62,24 @@ void check_irq_resend(struct irq_desc *d
         */
        desc->chip->enable(irq);
 
+       /*
+        * Temporary hack to figure out more about the problem, which
+        * is causing the ancient network cards to die.
+        */
+
        if ((status & (IRQ_PENDING | IRQ_REPLAY)) == IRQ_PENDING) {
                desc->status = (status & ~IRQ_PENDING) | IRQ_REPLAY;
 
-               if (!desc->chip || !desc->chip->retrigger ||
-                                       !desc->chip->retrigger(irq)) {
+               if (desc->handle_irq == handle_edge_irq) {
+                       if (desc->chip->retrigger)
+                               desc->chip->retrigger(irq);
+                       return;
+               }
 #ifdef CONFIG_HARDIRQS_SW_RESEND
-                       /* Set it pending and activate the softirq: */
-                       set_bit(irq, irqs_resend);
-                       tasklet_schedule(&resend_tasklet);
+               WARN_ON_ONCE(1);
+               /* Set it pending and activate the softirq: */

kernel/irq/resend.c 2007-08-07 13:57:54.000000000 +0200 void check_irq_resend(struct irq_desc *d · line 85

Quoted 7 lines

kernel/irq/resend.c 2007-08-07 13:57:54.000000000 +0200 renamed from kernel/irq/resend.c 2007-07-09 01:32:17.000000000 +0200 +2 -1

+               set_bit(irq, irqs_resend);
+               tasklet_schedule(&resend_tasklet);
 #endif
-               }
        }
 }

Works fine with:

WARNING: at kernel/irq/resend.c:79 check_irq_resend()

Call Trace:
 [<ffffffff8025e660>] check_irq_resend+0xc0/0xd0
 [<ffffffff8025e1cd>] enable_irq+0xed/0xf0
 [<ffffffff8807f21d>] :8390:ei_start_xmit+0x14d/0x30c
 [<ffffffff8024d055>] lock_release_non_nested+0xe5/0x190
 [<ffffffff80539b78>] __qdisc_run+0x98/0x1f0
 [<ffffffff80539b8e>] __qdisc_run+0xae/0x1f0
 [<ffffffff8052b65e>] dev_hard_start_xmit+0x26e/0x2d0
 [<ffffffff80539ba0>] __qdisc_run+0xc0/0x1f0
 [<ffffffff8052dc2f>] dev_queue_xmit+0x24f/0x310
 [<ffffffff805337a7>] neigh_resolve_output+0xe7/0x290
 [<ffffffff8054f5c0>] dst_output+0x0/0x10
 [<ffffffff80552aff>] ip_output+0x19f/0x340
 [<ffffffff80551f77>] ip_queue_xmit+0x217/0x430
 [<ffffffff80563b2a>] tcp_transmit_skb+0x40a/0x7c0
 [<ffffffff805657bb>] __tcp_push_pending_frames+0x11b/0x940

 [<ffffffff8055972a>] tcp_sendmsg+0x87a/0xc80
 [<ffffffff80577735>] inet_sendmsg+0x45/0x80
 [<ffffffff8051e2d4>] sock_aio_write+0x104/0x120
 [<ffffffff80285fc1>] do_sync_write+0xf1/0x130
 [<ffffffff80243290>] autoremove_wake_function+0x0/0x40
 [<ffffffff802868e9>] vfs_write+0x159/0x170
 [<ffffffff80286ef0>] sys_write+0x50/0x90
 [<ffffffff802097fe>] system_call+0x7e/0x83

So, it looks like x86_64 io_apic's IPI code was unused too long...

Read below please:

Quoted 5 lines · Marcin Ślusarz

2007/8/7, Jarek Poplawski <[email protected]>:

Quoted 4 lines

So, the let's try this idea yet: modified Ingo's "x86: activate HARDIRQS_SW_RESEND" patch.
(Don't forget about make oldconfig before make.)
For testing only.

Quoted 8 lines

Quoted 8 lines

diff -Nurp 2.6.22.1-/arch/i386/Kconfig 2.6.22.1/arch/i386/Kconfig

arch/i386/Kconfig 2007-08-07 13:13:03.000000000 +0200 renamed from arch/i386/Kconfig 2007-07-09 01:32:17.000000000 +0200 +1 -0

@@ -1252,6 +1252,10 @@ config GENERIC_PENDING_IRQ
        depends on GENERIC_HARDIRQS && SMP
        default y
 
+config HARDIRQS_SW_RESEND

Quoted 1 line

Works fine with:

Very nice! It would be about time this kernel should start behave...

Quoted 3 lines

WARNING: at kernel/irq/resend.c:79 check_irq_resend()

Call Trace:

So, it looks like x86_64 io_apic's IPI code was unused too long... I hope it's a piece of cake for Ingo now...

2007/8/9, Jarek Poplawski <[email protected]>:

Quoted 9 lines

diff -Nurp 2.6.23-rc1-/kernel/irq/chip.c 2.6.23-rc1/kernel/irq/chip.c

kernel/irq/chip.c 2007-08-08 20:49:07.000000000 +0200 renamed from kernel/irq/chip.c 2007-07-09 01:32:17.000000000 +0200 +1 -0

@@ -389,12 +389,19 @@ handle_fasteoi_irq(unsigned int irq, str
        unsigned int cpu = smp_processor_id();
        struct irqaction *action;
        irqreturn_t action_ret;
+       int edge = 0;
 

NETDEV WATCHDOG: eth0: transmit timed out
eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=351.
eth0: Resetting the 8390 t=4295229000...<6>NETDEV WATCHDOG: eth0:
transmit timed out
eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=718.
eth0: Resetting the 8390 t=4295230000...<6>NETDEV WATCHDOG: eth0:
transmit timed out
eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=874. etc...

(..)
I think, there is this one possible for your testing yet?:
Subject: [patch] genirq: temporary fix for level-triggered IRQ resend
Date: Wed, 8 Aug 2007 13:00:37 +0200

If it's not a great problem it would be interesting to try this with different CONFIG_HZ too e.g. you could start with 100 (I guess, you tested very similar thing in 2.6.23-rc2 with 1000(?) already).

2007/8/10, Jarek Poplawski <[email protected]>:

Quoted 4 lines

(..)
I think, there is this one possible for your testing yet?:
Subject: [patch] genirq: temporary fix for level-triggered IRQ resend
Date: Wed, 8 Aug 2007 13:00:37 +0200

I think I already tested this patch, but this thread is sooo big and I can't find my response...

Quoted 4 lines

If it's not a great problem it would be interesting to try this with different CONFIG_HZ too e.g. you could start with 100 (I guess, you tested very similar thing in 2.6.23-rc2 with 1000(?) already).

My all tests were done on 2.6.22.1

Linus,

with -rc2 approaching i think we should apply the minimal fix below to get Marcin's ne2k-pci networking back in working order. The WARN_ON_ONCE() will not prevent the system from working and it will be a reminder.

Ok the logic behind the 8390 is very simple:

Things to know
- IRQ delivery is asynchronous to the PCI bus
- Blocking the local CPU IRQ via spin locks was too slow
- The chip has register windows needing locking work

So the path was once (I say once as people appear to have changed it in the mean time and it now looks rather bogus if the changes to use disable_irq_nosync_irqsave are disabling the local IRQ)

Take the page lock
Mask the IRQ on chip
Disable the IRQ (but not mask locally- someone seems to have
broken this with the lock validator stuff) [This must be _nosync as the page lock may otherwise
deadlock us]

Drop the page lock and turn IRQs back on

At this point an existing IRQ may still be running but we can't get a new one

Take the lock (so we know the IRQ has terminated) but don't mask
the IRQs on the processor
Set irqlock [for debug]

Transmit (slow as ****)

re-enable the IRQ

We have to use disable_irq because otherwise you will get delayed interrupts on the APIC bus deadlocking the transmit path.

Quite hairy but the chip simply wasn't designed for SMP and you can't even ACK an interrupt without risking corrupting other parallel activities on the chip.

Linus,

with -rc2 approaching i think we should apply the minimal fix below to get Marcin's ne2k-pci networking back in working order. The WARN_ON_ONCE() will not prevent the system from working and it will be a reminder.

a better workaround would be to inhibit the resent vector via the IO-APIC irqchip - but i'd still like to have the patch below because the ne2k driver _should_ be able to survive the spurious irq that happens. (even on Marcin's system that ne2k-pci irq line is shared with another networking card, so an irq could happen at any moment - it's just that with the delayed-disable logic it happens _all the time_.)

Ingo

I get a warning on each boot now with this patch ..

[   63.686613] WARNING: at kernel/irq/resend.c:70 check_irq_resend()
[   63.686636]  [<c013c55c>] check_irq_resend+0x8c/0xa0
[   63.686653]  [<c013c15f>] enable_irq+0xad/0xb3
[   63.686662]  [<e886481e>] vortex_timer+0x20c/0x3d5 [3c59x]
[   63.686675]  [<c01164b9>] scheduler_tick+0x154/0x273
[   63.686685]  [<c012fed1>] getnstimeofday+0x34/0xe3
[   63.686697]  [<c0121f4a>] run_timer_softirq+0x137/0x197
[   63.686709]  [<e8864612>] vortex_timer+0x0/0x3d5 [3c59x]
[   63.686720]  [<c011ed09>] __do_softirq+0x75/0xe1
[   63.686729]  [<c011edac>] do_softirq+0x37/0x3d
[   63.686735]  [<c011ef85>] irq_exit+0x7c/0x7e
[   63.686740]  [<c010e013>] smp_apic_timer_interrupt+0x59/0x84
[   63.686751]  [<c0103428>] apic_timer_interrupt+0x28/0x30
[   63.686759]  [<c0101355>] default_idle+0x0/0x3f
[   63.686767]  [<c0101385>] default_idle+0x30/0x3f
[   63.686773]  [<c0100c19>] cpu_idle+0x5e/0x8e
[   63.686779]  [<c03fdc5f>] start_kernel+0x2d7/0x368

That means ?:)

Quoted 3 lines · Gabriel C

I get a warning on each boot now with this patch ..

[   63.686613] WARNING: at kernel/irq/resend.c:70 check_irq_resend()

we are still trying to figure out what happens with ne2k-pci. The message will vanish soon.

I can't guarantee this is all needed to fix this bug, but I think this patch is necessary here.

Quoted 2 lines · Jarek Poplawski

I can't guarantee this is all needed to fix this bug, but I think this patch is necessary here.

hmmm ... very interesting! Now _this_ is something we'd like to see tested. Could you send a patch to Marcin that also undoes the workaround we have in place now, so that he could check whether ne2k-pci works fine with your fix alone?

Quoted 2 lines · Jarek Poplawski

I can't guarantee this is all needed to fix this bug, but I think this patch is necessary here.

hmmm ... very interesting! Now _this_ is something we'd like to see tested. Could you send a patch to Marcin that also undoes the workaround we have in place now, so that he could check whether ne2k-pci works fine with your fix alone?

Quoted 10 lines · Jarek Poplawski

Quoted 3 lines

I can't guarantee this is all needed to fix this bug, but I think this patch is necessary here.

hmmm ... very interesting! Now _this_ is something we'd like to see tested. Could you send a patch to Marcin that also undoes the workaround we have in place now, so that he could check whether ne2k-pci works fine with your fix alone?

I'm not sure this is needed... Marcin got this patch, I hope, and I don't have another possibility to contact with him. Since he managed with this bisection and all the previous patches I don't think there could be any problems, so:

Marcin! I'd be very glad if you could test this patch alone; this should apply without any problems to 2.6.21 (with some offset) and later "vanilla" versions (or try to revert Ingo's last patch with patch -p1 -R). Please, contact me on any problems (alas not during the weekend...).

I'll test this patch tomorrow (and confirm that the last one from Ingo works fine) and report results on monday (sorry, no internet at home since I moved out of city :|).

Hi,

I can't guarantee this is all needed to fix this bug, but I think this patch is necessary here.

Regards,
Jarek P.
------------>

Subject: genirq: fix simple and fasteoi irq handlers

After the "genirq: do not mask interrupts by default" patch interrupts should be disabled not immediately upon request, but after they happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a driver's work.

The main reason of problems here, pointing the broken patch and making the first patch which can fix this was done by Marcin Slusarz. Additional test patches of Thomas Gleixner and Ingo Molnar tested by Marcin Slusarz helped to narrow possible reasons even more. Thanks.

PS: this patch fixes only one evident error here, but there could be more places affected by above-mentioned change in irq handling.

Signed-off-by Jarek Poplawski <[email protected]>

---

diff -Nurp 2.6.23-rc1-/kernel/irq/chip.c 2.6.23-rc1/kernel/irq/chip.c

kernel/irq/chip.c 2007-08-02 20:42:38.000000000 +0200 renamed from kernel/irq/chip.c 2007-07-09 01:32:17.000000000 +0200 +2 -7

@@ -295,12 +295,11 @@ handle_simple_irq(unsigned int irq, stru
 
        spin_lock(&desc->lock);
 
-       if (unlikely(desc->status & IRQ_INPROGRESS))
-               goto out_unlock;
        kstat_cpu(cpu).irqs[irq]++;
 
        action = desc->action;
-       if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+       if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
+                                                IRQ_DISABLED)))) {
                if (desc->chip->mask)
                        desc->chip->mask(irq);
                desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);

@@ -392,18 +391,16 @@ handle_fasteoi_irq(unsigned int irq, str
 
        spin_lock(&desc->lock);
 
-       if (unlikely(desc->status & IRQ_INPROGRESS))
-               goto out;
-
        desc->status &= ~(IRQ_REPLAY | IRQ_WAITING);
        kstat_cpu(cpu).irqs[irq]++;
 
        /*
-        * If its disabled or no action available

Subject: genirq: fix simple and fasteoi irq handlers

After the "genirq: do not mask interrupts by default" patch interrupts should be disabled not immediately upon request, but after they happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a driver's work.

Quoted 8 lines

Subject: genirq: fix simple and fasteoi irq handlers

After the "genirq: do not mask interrupts by default" patch interrupts should be disabled not immediately upon request, but after they happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a driver's work.

nice fix. I think this is exactly the type of bug we were hoping to be able to identify and fix, and it could explain the regression in its entirety. The big question - does it fix Marcin's regression?

Quoted 7 lines · Jarek Poplawski

Subject: genirq: fix simple and fasteoi irq handlers

After the "genirq: do not mask interrupts by default" patch interrupts should be disabled not immediately upon request, but after they happen. But, handle_simple_irq() and handle_fasteoi_irq() can skip this once or more if an irq is just serviced (IRQ_INPROGRESS), possibly disrupting a driver's work.

nice fix. I think this is exactly the type of bug we were hoping to be able to identify and fix, and it could explain the regression in its entirety. The big question - does it fix Marcin's regression?

On Thu, 2007-07-26 at 10:13 +0200, Jarek Poplawski wrote:

Quoted 3 lines

PS: Now, it seems to me Thomas could be the nearest. BTW, could somebody give me some tip, how these re-triggered interrupts are skipped on dev's reset before enable_irq?

I think the correct solution is really not to resend level type interrupts. If the interrupt line is still active, then the interrupt comes up by itself. I'm cooking a patch for that.

The other question is:

Is the driver confused by the resent irq or is the chip-set unhappy about the resend ?

We could figure the latter out by activating the software based resend method.

2007/7/25, Thomas Gleixner <[email protected]>:

Quoted 1 line

(...)

I've tested Jarek's patch, 2 Ingo's patches (2nd and 3rd) and Thomas' patch (one patch at time of course) - all of them fixed the problem, but the last one flooded my logs with "Skip resend for irq 17". All tests were done on 2.6.21.3.