[BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[M.H.VanLeeuwen]

Hi,

I'm seeing an NMI Watchdog detected LOCKUP go away when I revert this patch
previously added into test8.

Any help appreciated.
Martin

diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
--- a/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
+++ b/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
@@ -378,7 +380,7 @@
        spin_lock_irqsave(&desc->lock, flags);
        switch (desc->depth) {
        case 1: {
-               unsigned int status = desc->status & ~IRQ_DISABLED;
+               unsigned int status = desc->status & ~(IRQ_DISABLED | IRQ_INPROGRESS);
                desc->status = status;
                if ((status & (IRQ_PENDING | IRQ_REPLAY)) == IRQ_PENDING) {
                        desc->status = status | IRQ_REPLAY;

EIP is at .text.lock.8390+0x39/0x63 which is in ei_start_xmit() in 8390.c
at the first spin_lock_irqsave().

I hand copied the data from the console, what else is interesting/necessary?

First notices after booting into test8 and the system went silent when starting X,
since /home is NFS mounted go generate network and IDE activity.

Reproducible by doing all 3 of these (any 2 and the system stays alive, longer
than I want to wait)

1. ping flood  A->B
2. ping flood  B->A
3. find and grep for garbage from IDE on B's /dev/md/X filesystem

System B is SMP dual Celeron 466Mhz.

Eth interface:

isapnp: Scanning for PnP cards...
isapnp: Card 'SMC EZ Card (1660)'
isapnp: 1 Plug & Play card detected total
pnp: Device 00:01.00 activated.
ne.c: ISAPnP reports Generic PNP at i/o 0x220, irq 5.
ne.c:v1.10 9/23/94 Donald Becker (becker@scyld.com)
Last modified Nov 1, 2000 by Paul Gortmaker
NE*000 ethercard probe at 0x220: 00 e0 29 3c 1f 11
eth0: NE2000 found at 0x220, using IRQ 5.


IDE interface:

PIIX4: IDE controller at PCI slot 0000:00:07.1
PIIX4: chipset revision 1
PIIX4: not 100% native mode: will probe irqs later
    ide0: BM-DMA at 0xf000-0xf007, BIOS settings: hda:pio, hdb:pio
    ide1: BM-DMA at 0xf008-0xf00f, BIOS settings: hdc:pio, hdd:pio
HPT366: onboard version of chipset, pin1=1 pin2=2
HPT366: IDE controller at PCI slot 0000:00:13.0
HPT366: chipset revision 1
HPT366: 100% native mode on irq 18
    ide2: BM-DMA at 0xdc00-0xdc07, BIOS settings: hde:pio, hdf:pio
    ide3: BM-DMA at 0xe800-0xe807, BIOS settings: hdg:pio, hdh:pio
hde: WDC WD400BB-32AUA1, ATA DISK drive
ide2 at 0xd400-0xd407,0xd802 on irq 18
hdg: ST340810A, ATA DISK drive
ide3 at 0xe000-0xe007,0xe402 on irq 18
hde: max request size: 128KiB
hde: 78165360 sectors (40020 MB) w/2048KiB Cache, CHS=65535/16/63, UDMA(66)
 /dev/ide/host2/bus0/target0/lun0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 >
hdg: max request size: 128KiB
hdg: 78165360 sectors (40020 MB) w/2048KiB Cache, CHS=65535/16/63, UDMA(66)
 /dev/ide/host3/bus0/target0/lun0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 >

/proc/interrupts (currently running 2.6.0-test8)

          CPU0       CPU1
  0:   52531411         84    IO-APIC-edge  timer
  1:      10288          1    IO-APIC-edge  i8042
  2:          0          0          XT-PIC  cascade
  5:    1908410   66015823    IO-APIC-edge  NE2000
  8:          1          0    IO-APIC-edge  rtc
 12:      56098          1    IO-APIC-edge  i8042
 16:    3663003          0   IO-APIC-level  r128@PCI:1:0:0
 18:     162982     300769   IO-APIC-level  ide2, ide3
NMI:   52531430   52531316
LOC:   52544445   52544450
ERR:         42
MIS:        999

Re: [BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[Linus Torvalds]

On Wed, 22 Oct 2003, M.H.VanLeeuwen wrote:
> 
> I'm seeing an NMI Watchdog detected LOCKUP go away when I revert this patch
> previously added into test8.

Yes, the thing is buggy. 

It's not correct for "disable_irq_nosync()" users, and reverting it is the 
right thing to do. Thanks for the report.

Marcelo, please note if you played with this in 2.4.x..

		Linus

------
> diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
> --- a/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
> +++ b/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
> @@ -378,7 +380,7 @@
>         spin_lock_irqsave(&desc->lock, flags);
>         switch (desc->depth) {
>         case 1: {
> -               unsigned int status = desc->status & ~IRQ_DISABLED;
> +               unsigned int status = desc->status & ~(IRQ_DISABLED | IRQ_INPROGRESS);
>                 desc->status = status;
>                 if ((status & (IRQ_PENDING | IRQ_REPLAY)) == IRQ_PENDING) {
>                         desc->status = status | IRQ_REPLAY;
> 
> EIP is at .text.lock.8390+0x39/0x63 which is in ei_start_xmit() in 8390.c
> at the first spin_lock_irqsave().
> 
> I hand copied the data from the console, what else is interesting/necessary?
> 
> First notices after booting into test8 and the system went silent when starting X,
> since /home is NFS mounted go generate network and IDE activity.
> 
> Reproducible by doing all 3 of these (any 2 and the system stays alive, longer
> than I want to wait)
> 
> 1. ping flood  A->B
> 2. ping flood  B->A
> 3. find and grep for garbage from IDE on B's /dev/md/X filesystem
> 
> System B is SMP dual Celeron 466Mhz.
> 
> Eth interface:
> 
> isapnp: Scanning for PnP cards...
> isapnp: Card 'SMC EZ Card (1660)'
> isapnp: 1 Plug & Play card detected total
> pnp: Device 00:01.00 activated.
> ne.c: ISAPnP reports Generic PNP at i/o 0x220, irq 5.
> ne.c:v1.10 9/23/94 Donald Becker (becker@scyld.com)
> Last modified Nov 1, 2000 by Paul Gortmaker
> NE*000 ethercard probe at 0x220: 00 e0 29 3c 1f 11
> eth0: NE2000 found at 0x220, using IRQ 5.
> 
> 
> IDE interface:
> 
> PIIX4: IDE controller at PCI slot 0000:00:07.1
> PIIX4: chipset revision 1
> PIIX4: not 100% native mode: will probe irqs later
>     ide0: BM-DMA at 0xf000-0xf007, BIOS settings: hda:pio, hdb:pio
>     ide1: BM-DMA at 0xf008-0xf00f, BIOS settings: hdc:pio, hdd:pio
> HPT366: onboard version of chipset, pin1=1 pin2=2
> HPT366: IDE controller at PCI slot 0000:00:13.0
> HPT366: chipset revision 1
> HPT366: 100% native mode on irq 18
>     ide2: BM-DMA at 0xdc00-0xdc07, BIOS settings: hde:pio, hdf:pio
>     ide3: BM-DMA at 0xe800-0xe807, BIOS settings: hdg:pio, hdh:pio
> hde: WDC WD400BB-32AUA1, ATA DISK drive
> ide2 at 0xd400-0xd407,0xd802 on irq 18
> hdg: ST340810A, ATA DISK drive
> ide3 at 0xe000-0xe007,0xe402 on irq 18
> hde: max request size: 128KiB
> hde: 78165360 sectors (40020 MB) w/2048KiB Cache, CHS=65535/16/63, UDMA(66)
>  /dev/ide/host2/bus0/target0/lun0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 >
> hdg: max request size: 128KiB
> hdg: 78165360 sectors (40020 MB) w/2048KiB Cache, CHS=65535/16/63, UDMA(66)
>  /dev/ide/host3/bus0/target0/lun0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 >
> 
> /proc/interrupts (currently running 2.6.0-test8)
> 
>           CPU0       CPU1
>   0:   52531411         84    IO-APIC-edge  timer
>   1:      10288          1    IO-APIC-edge  i8042
>   2:          0          0          XT-PIC  cascade
>   5:    1908410   66015823    IO-APIC-edge  NE2000
>   8:          1          0    IO-APIC-edge  rtc
>  12:      56098          1    IO-APIC-edge  i8042
>  16:    3663003          0   IO-APIC-level  r128@PCI:1:0:0
>  18:     162982     300769   IO-APIC-level  ide2, ide3
> NMI:   52531430   52531316
> LOC:   52544445   52544450
> ERR:         42
> MIS:        999
> 

Re: [BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[Linus Torvalds]

On Wed, 22 Oct 2003, Linus Torvalds wrote:
> 
> It's not correct for "disable_irq_nosync()" users, and reverting it is the 
> right thing to do. Thanks for the report.

[ Btw, the 8390-based network drivers are likely the only thing to
  actually be able to trigger it, so thanks again for the good bug report. ]

		Linus

Re: [BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[Marcelo Tosatti]

On Wed, 22 Oct 2003, Linus Torvalds wrote:

> 
> On Wed, 22 Oct 2003, M.H.VanLeeuwen wrote:
> > 
> > I'm seeing an NMI Watchdog detected LOCKUP go away when I revert this patch
> > previously added into test8.
> 
> Yes, the thing is buggy. 
> 
> It's not correct for "disable_irq_nosync()" users, and reverting it is the 
> right thing to do. Thanks for the report.
> 
> Marcelo, please note if you played with this in 2.4.x..

2.4.23-pre8 does clear IRQ_INPROGRESS in setup_IRQ() (it fixes the IDE setup
hang viro mentioned). 

disable_irq_nosync() doesnt care about IRQ_INPROGRESS. I cant figure out why 
does the change breaks...
 

Re: [BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[Mikael Pettersson]

Linus Torvalds writes:
 > On Wed, 22 Oct 2003, M.H.VanLeeuwen wrote:
 > > 
 > > I'm seeing an NMI Watchdog detected LOCKUP go away when I revert this patch
 > > previously added into test8.
 > 
 > Yes, the thing is buggy. 
 > 
 > It's not correct for "disable_irq_nosync()" users, and reverting it is the 
 > right thing to do. Thanks for the report.
 > 
 > Marcelo, please note if you played with this in 2.4.x..
 > 
 > 		Linus
 > 
 > ------
 > > diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
 > > --- a/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
 > > +++ b/arch/i386/kernel/irq.c    Fri Oct 17 14:43:50 2003
 > > @@ -378,7 +380,7 @@
 > >         spin_lock_irqsave(&desc->lock, flags);
 > >         switch (desc->depth) {
 > >         case 1: {
 > > -               unsigned int status = desc->status & ~IRQ_DISABLED;
 > > +               unsigned int status = desc->status & ~(IRQ_DISABLED | IRQ_INPROGRESS);

It seems 2.4.23-pre8 included something like this apparently broken
change (see diff from -pre7 below). Should it be reverted?

--- linux-2.4.23-pre7/arch/i386/kernel/irq.c	2003-10-23 13:17:43.700067608 +0200
+++ linux-2.4.23-pre8/arch/i386/kernel/irq.c	2003-10-23 13:17:10.071202512 +0200
@@ -1036,7 +1036,7 @@
 
 	if (!shared) {
 		desc->depth = 0;
-		desc->status &= ~(IRQ_DISABLED | IRQ_AUTODETECT | IRQ_WAITING);
+		desc->status &= ~(IRQ_DISABLED | IRQ_AUTODETECT | IRQ_WAITING | IRQ_INPROGRESS);
 		desc->handler->startup(irq);
 	}
 	spin_unlock_irqrestore(&desc->lock,flags);

Re: [BUG somewhere] 2.6.0-test8 irq.c, IRQ_INPROGRESS ?

[Linus Torvalds]

On Thu, 23 Oct 2003, Mikael Pettersson wrote:
> 
> It seems 2.4.23-pre8 included something like this apparently broken
> change (see diff from -pre7 below). Should it be reverted?

No, that one is correct. IRQ_INPROGRESS should indeed be cleared when the 
first handler is installed. It's only clearing it at enable_irq() that is 
wrong.

Also, the "disable_irq()" function _should_ look something like this:

	void disable_irq(unsigned int irq)
	{
	        irq_desc_t *desc = irq_desc + irq;
	        disable_irq_nosync(irq);
	        if (desc->action)
	                synchronize_irq(irq);
	}

ie it should only do synchronize_irq() if a handler exists. That fixes a
potential problem with drivers doing multiple disable_irq()/enable_irq()  
while no handler has been assigned yet.

		Linus

[PATCH] 2.6.0-test8 ISAPNP ne.c initialization

[M.H.VanLeeuwen]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

Hi,

 The level of isapnp_init was moved to after apci sometime ago.  Since it is now
 after net_dev_init, ISA PNP NICs fail to initialized at boot.  This is particularily
 problematic for NFS root filesystems like mine, or none modular systems.
 I've been patching my kernel since at least 2.5.65 with the attached...

 This fix allows ISA PNP NIC cards to work during net_dev_init, and still
 leaves isapnp_init after apci_init, though I don't know if it is the right
 thing todo, it works for me...

Comments from Alan Cox a long while back:
>We must initialise ACPI before ISAPnP because we need PCI and ACPI to
>know what system resources we must not hit. How about moving the
>net_dev_init to later?

Here is the ordering of initcall from System.map file w/ my changes
on 2.6.0-test8.

c044a700 t __initcall_acpi_pci_link_init
c044a704 t __initcall_acpi_power_init
c044a708 t __initcall_acpi_system_init
c044a70c t __initcall_acpi_event_init
c044a710 t __initcall_acpi_scan_init
c044a714 t __initcall_pnp_init
c044a718 t __initcall_pnp_system_init
c044a71c t __initcall_device_init
c044a720 t __initcall_as_slab_setup
c044a724 t __initcall_deadline_slab_setup
c044a728 t __initcall_input_init
c044a72c t __initcall_i2c_init
c044a730 t __initcall_pci_acpi_init
c044a734 t __initcall_pci_legacy_init
c044a738 t __initcall_pcibios_irq_init
c044a73c t __initcall_pcibios_init
c044a740 t __initcall_isapnp_init
c044a744 t __initcall_chr_dev_init
c044a748 t __initcall_net_dev_init
c044a74c t __initcall_i8259A_init_sysfs
c044a750 t __initcall_init_timer_sysfs
c044a754 t __initcall_time_init_device


I assume System.map is the place to see initcall ordering?

Martin

[-- Attachment #2: test8.isapnp.patch --]
[-- Type: text/plain, Size: 577 bytes --]

--- linux-2.6.0-test8.virgin/drivers/pnp/isapnp/core.c	Sat Sep 27 23:17:52 2003
+++ linux-2.6.0-test8/drivers/pnp/isapnp/core.c	Sat Sep 27 23:20:18 2003
@@ -1160,7 +1160,7 @@
 	return 0;
 }
 
-device_initcall(isapnp_init);
+fs_initcall(isapnp_init);
 
 /* format is: noisapnp */
 
--- linux-2.6.0-test8.virgin/net/core/dev.c	Sat Oct 18 09:36:27 2003
+++ linux-2.6.0-test8/net/core/dev.c	Wed Oct 22 23:22:57 2003
@@ -3018,7 +3018,7 @@
 	return rc;
 }
 
-subsys_initcall(net_dev_init);
+fs_initcall(net_dev_init);
 
 EXPORT_SYMBOL(__dev_get);
 EXPORT_SYMBOL(__dev_get_by_flags);

Re: [PATCH] 2.6.0-test8 ISAPNP ne.c initialization

[Adam Belay]

On Wed, Oct 22, 2003 at 11:30:39PM -0500, M.H.VanLeeuwen wrote:
> Hi,
> 
>  The level of isapnp_init was moved to after apci sometime ago.  Since it is now
>  after net_dev_init, ISA PNP NICs fail to initialized at boot.  This is particularily
>  problematic for NFS root filesystems like mine, or none modular systems.
>  I've been patching my kernel since at least 2.5.65 with the attached...
> 
>  This fix allows ISA PNP NIC cards to work during net_dev_init, and still
>  leaves isapnp_init after apci_init, though I don't know if it is the right
>  thing todo, it works for me...
> 
> Comments from Alan Cox a long while back:
> >We must initialise ACPI before ISAPnP because we need PCI and ACPI to
> >know what system resources we must not hit. How about moving the
> >net_dev_init to later?
>

-->snip

> -device_initcall(isapnp_init);
> +fs_initcall(isapnp_init);

I'm concerned that moving isapnp further down will spark some new problems.
Also this would be far later in the init cycle than most buses.

-->snip

> -subsys_initcall(net_dev_init);
> +fs_initcall(net_dev_init);

-->snip

How about something like this? (untested)

The patch removes the legacy probing function from dev.c and gives it its
own initcall later in the cycle.  Any testing would be appreciated.  I
also have some patches that update the probing code in some of these
drivers so that they don't have to use legacy techniques but they need
to be updated to test9.  This fix is probably the least intrusive.

--- a/drivers/net/Space.c	2003-10-25 18:42:56.000000000 +0000
+++ b/drivers/net/Space.c	2003-11-01 22:15:01.000000000 +0000
@@ -422,7 +422,7 @@
 extern int loopback_init(void);

 /*  Statically configured drivers -- order matters here. */
-void __init probe_old_netdevs(void)
+int __init net_olddevs_init(void)
 {
 	int num;

@@ -450,8 +450,12 @@
 #ifdef CONFIG_LTPC
 	ltpc_probe();
 #endif
+
+	return 0;
 }

+device_initcall(net_olddevs_init);
+
 /*
  * The @dev_base list is protected by @dev_base_lock and the rtln
  * semaphore.
--- a/include/linux/netdevice.h	2003-10-25 18:44:45.000000000 +0000
+++ b/include/linux/netdevice.h	2003-11-01 22:11:04.000000000 +0000
@@ -494,7 +494,6 @@
 extern struct net_device		*dev_base;		/* All devices */
 extern rwlock_t				dev_base_lock;		/* Device list lock */

-extern void		probe_old_netdevs(void);
 extern int			netdev_boot_setup_add(char *name, struct ifmap *map);
 extern int 			netdev_boot_setup_check(struct net_device *dev);
 extern struct net_device    *dev_getbyhwaddr(unsigned short type, char *hwaddr);
--- a/net/core/dev.c	2003-10-25 18:43:39.000000000 +0000
+++ b/net/core/dev.c	2003-11-02 16:10:51.000000000 +0000
@@ -3007,8 +3007,6 @@

 	dev_boot_phase = 0;

-	probe_old_netdevs();
-
 	open_softirq(NET_TX_SOFTIRQ, net_tx_action, NULL);
 	open_softirq(NET_RX_SOFTIRQ, net_rx_action, NULL);

Re: [PATCH] 2.6.0-test8 ISAPNP ne.c initialization

[M.H.VanLeeuwen]

Adam Belay wrote:
> 
> How about something like this? (untested)

I had the same reservation with re-ordering inits...

Removed my patch, added this one, compiled and booted; works for me.

Thanks,
Martin

> 
> The patch removes the legacy probing function from dev.c and gives it its
> own initcall later in the cycle.  Any testing would be appreciated.  I
> also have some patches that update the probing code in some of these
> drivers so that they don't have to use legacy techniques but they need
> to be updated to test9.  This fix is probably the least intrusive.
> 
> --- a/drivers/net/Space.c       2003-10-25 18:42:56.000000000 +0000
> +++ b/drivers/net/Space.c       2003-11-01 22:15:01.000000000 +0000
> @@ -422,7 +422,7 @@
>  extern int loopback_init(void);
> 
>  /*  Statically configured drivers -- order matters here. */
> -void __init probe_old_netdevs(void)
> +int __init net_olddevs_init(void)
>  {
>         int num;
> 
> @@ -450,8 +450,12 @@
>  #ifdef CONFIG_LTPC
>         ltpc_probe();
>  #endif
> +
> +       return 0;
>  }
> 
> +device_initcall(net_olddevs_init);
> +
>  /*
>   * The @dev_base list is protected by @dev_base_lock and the rtln
>   * semaphore.
> --- a/include/linux/netdevice.h 2003-10-25 18:44:45.000000000 +0000
> +++ b/include/linux/netdevice.h 2003-11-01 22:11:04.000000000 +0000
> @@ -494,7 +494,6 @@
>  extern struct net_device               *dev_base;              /* All devices */
>  extern rwlock_t                                dev_base_lock;          /* Device list lock */
> 
> -extern void            probe_old_netdevs(void);
>  extern int                     netdev_boot_setup_add(char *name, struct ifmap *map);
>  extern int                     netdev_boot_setup_check(struct net_device *dev);
>  extern struct net_device    *dev_getbyhwaddr(unsigned short type, char *hwaddr);
> --- a/net/core/dev.c    2003-10-25 18:43:39.000000000 +0000
> +++ b/net/core/dev.c    2003-11-02 16:10:51.000000000 +0000
> @@ -3007,8 +3007,6 @@
> 
>         dev_boot_phase = 0;
> 
> -       probe_old_netdevs();
> -
>         open_softirq(NET_TX_SOFTIRQ, net_tx_action, NULL);
>         open_softirq(NET_RX_SOFTIRQ, net_rx_action, NULL);