* 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
@ 2004-09-27 19:24 Paul Fulghum
2004-09-27 21:12 ` Paul Fulghum
2004-09-27 23:00 ` J.A. Magallon
0 siblings, 2 replies; 10+ messages in thread
From: Paul Fulghum @ 2004-09-27 19:24 UTC (permalink / raw)
To: scott.feldman; +Cc: linux-kernel
The e100 module is generating a warning:
Sep 27 13:30:29 deimos kernel: e100: Intel(R) PRO/100 Network Driver, 3.1.4-NAPI
Sep 27 13:30:29 deimos kernel: e100: Copyright(c) 1999-2004 Intel Corporation
Sep 27 13:30:29 deimos kernel: e100: eth0: e100_probe: addr 0xfecfc000, irq 16, MAC addr 00:90:27:3A:C5:E3
Sep 27 13:30:29 deimos kernel: enable_irq(16) unbalanced from ec83ff33
Sep 27 13:30:29 deimos kernel: [<c010923f>] enable_irq+0xcf/0xe0
Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
Sep 27 13:30:29 deimos kernel: [<ec83f410>] e100_intr+0x0/0x140 [e100]
Sep 27 13:30:29 deimos kernel: [<ec841131>] e100_open+0x31/0x80 [e100]
Sep 27 13:30:29 deimos kernel: [<c0318d4c>] dev_open+0x8c/0xa0
Sep 27 13:30:29 deimos kernel: [<c031cc74>] dev_mc_upload+0x24/0x40
Sep 27 13:30:29 deimos kernel: [<c031a4ea>] dev_change_flags+0x12a/0x150
Sep 27 13:30:29 deimos kernel: [<c0318c0d>] dev_load+0x2d/0x80
Sep 27 13:30:29 deimos kernel: [<c0355b37>] devinet_ioctl+0x277/0x730
e100_up calls disable_irq, request_irq, then enable_irq
as shown below.
static int e100_up(struct nic *nic)
{
...
disable_irq(nic->pdev->irq);
...
if((err = request_irq(nic->pdev->irq, e100_intr, SA_SHIRQ,
nic->netdev->name, nic->netdev)))
goto err_no_irq;
e100_enable_irq(nic);
enable_irq(nic->pdev->irq);
netif_wake_queue(nic->netdev);
return 0;
...
}
On this machine, the e100 is the only device on that IRQ.
request_irq calls setup_irq which clears the irq descriptor
depth member to 0 and enables the interrupt because this
is the first device to use that interrupt.
This results in the warning on the next enable_irq().
I'm not sure why the driver is calling disable_irq
IRQ before calling request_irq. You can't get that
interrupt until you call request_irq, and once you
call request_irq you can (at least when this is
the first device on that IRQ) even before the
call to enable_irq.
I suspect the correct thing is to remove
disable_irq/enable_irq from e100_up.
I don't see any purpose for these calls in e100_up.
--
Paul Fulghum
paulkf@microgate.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-27 19:24 2.6.9-rc2-mm4 e100 enable_irq unbalanced from Paul Fulghum
@ 2004-09-27 21:12 ` Paul Fulghum
2004-09-30 18:21 ` Jeremy Fitzhardinge
2004-09-27 23:00 ` J.A. Magallon
1 sibling, 1 reply; 10+ messages in thread
From: Paul Fulghum @ 2004-09-27 21:12 UTC (permalink / raw)
To: scott.feldman; +Cc: linux-kernel
On Mon, 2004-09-27 at 14:24, Paul Fulghum wrote:
> The e100 module is generating a warning:
>
> Sep 27 13:30:29 deimos kernel: e100: Intel(R) PRO/100 Network Driver, 3.1.4-NAPI
> Sep 27 13:30:29 deimos kernel: e100: Copyright(c) 1999-2004 Intel Corporation
> Sep 27 13:30:29 deimos kernel: e100: eth0: e100_probe: addr 0xfecfc000, irq 16, MAC addr 00:90:27:3A:C5:E3
> Sep 27 13:30:29 deimos kernel: enable_irq(16) unbalanced from ec83ff33
> Sep 27 13:30:29 deimos kernel: [<c010923f>] enable_irq+0xcf/0xe0
> Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
The following patch works for me and removes the warning.
The disable_irq/enable_irq is not needed because
the ISR can't be called before calling request_irq,
the hardware is initialized before calling request_irq,
and request_irq itself enables the interrupt if needed.
Comments?
--
Paul Fulghum
paulkf@microgate.com
--- a/drivers/net/e100.c 2004-09-27 09:57:35.000000000 -0500
+++ b/drivers/net/e100.c 2004-09-27 16:00:12.115482112 -0500
@@ -1675,9 +1675,6 @@
if((err = e100_rx_alloc_list(nic)))
return err;
-
- disable_irq(nic->pdev->irq);
-
if((err = e100_alloc_cbs(nic)))
goto err_rx_clean_list;
if((err = e100_hw_init(nic)))
@@ -1689,7 +1686,6 @@
nic->netdev->name, nic->netdev)))
goto err_no_irq;
e100_enable_irq(nic);
- enable_irq(nic->pdev->irq);
netif_wake_queue(nic->netdev);
return 0;
@@ -1700,7 +1696,6 @@
err_rx_clean_list:
e100_rx_clean_list(nic);
- enable_irq(nic->pdev->irq);
return err;
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-27 19:24 2.6.9-rc2-mm4 e100 enable_irq unbalanced from Paul Fulghum
2004-09-27 21:12 ` Paul Fulghum
@ 2004-09-27 23:00 ` J.A. Magallon
2004-09-27 23:09 ` Paul Fulghum
1 sibling, 1 reply; 10+ messages in thread
From: J.A. Magallon @ 2004-09-27 23:00 UTC (permalink / raw)
To: linux-kernel
On 2004.09.27, Paul Fulghum wrote:
> The e100 module is generating a warning:
>
> Sep 27 13:30:29 deimos kernel: e100: Intel(R) PRO/100 Network Driver, 3.1.4-NAPI
> Sep 27 13:30:29 deimos kernel: e100: Copyright(c) 1999-2004 Intel Corporation
> Sep 27 13:30:29 deimos kernel: e100: eth0: e100_probe: addr 0xfecfc000, irq 16, MAC addr 00:90:27:3A:C5:E3
> Sep 27 13:30:29 deimos kernel: enable_irq(16) unbalanced from ec83ff33
> Sep 27 13:30:29 deimos kernel: [<c010923f>] enable_irq+0xcf/0xe0
> Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
> Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
> Sep 27 13:30:29 deimos kernel: [<ec83f410>] e100_intr+0x0/0x140 [e100]
> Sep 27 13:30:29 deimos kernel: [<ec841131>] e100_open+0x31/0x80 [e100]
> Sep 27 13:30:29 deimos kernel: [<c0318d4c>] dev_open+0x8c/0xa0
> Sep 27 13:30:29 deimos kernel: [<c031cc74>] dev_mc_upload+0x24/0x40
> Sep 27 13:30:29 deimos kernel: [<c031a4ea>] dev_change_flags+0x12a/0x150
> Sep 27 13:30:29 deimos kernel: [<c0318c0d>] dev_load+0x2d/0x80
> Sep 27 13:30:29 deimos kernel: [<c0355b37>] devinet_ioctl+0x277/0x730
>
Just a 'me-too', with a slightly different oops:
e100: Intel(R) PRO/100 Network Driver, 3.1.4-NAPI
e100: Copyright(c) 1999-2004 Intel Corporation
ACPI: PCI interrupt 0000:00:0d.0[A] -> GSI 19 (level, low) -> IRQ 185
e100: eth0: e100_probe: addr 0xf7161000, irq 185, MAC addr 00:30:48:41:22:9F
enable_irq(185) unbalanced from f89b3e25
[<c0108973>] enable_irq+0xa3/0xf0
[<f89b3e25>] e100_up+0xd5/0x1e0 [e100]
[<f89b3e25>] e100_up+0xd5/0x1e0 [e100]
[<f89b4e9a>] e100_open+0x2a/0x90 [e100]
[<c0368a24>] dev_open+0x74/0x90
[<c0369f66>] dev_change_flags+0x56/0x130
[<c03a2f42>] devinet_ioctl+0x5f2/0x6a0
[<c03a4e5f>] inet_ioctl+0xdf/0x100
[<c0360563>] sock_ioctl+0x1b3/0x270
[<c015a709>] fget+0x49/0x60
[<c016b905>] sys_ioctl+0x205/0x270
[<c0105b0d>] sysenter_past_esp+0x52/0x71
e100: eth0: e100_watchdog: link up, 100Mbps, full-duplex
--
J.A. Magallon <jamagallon()able!es> \ Software is like sex:
werewolf!able!es \ It's better when it's free
Mandrakelinux release 10.1 (Community) for i586
Linux 2.6.9-rc2-mm4 (gcc 3.4.1 (Mandrakelinux (Alpha 3.4.1-3mdk)) #2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-27 23:00 ` J.A. Magallon
@ 2004-09-27 23:09 ` Paul Fulghum
2004-09-28 22:13 ` J.A. Magallon
0 siblings, 1 reply; 10+ messages in thread
From: Paul Fulghum @ 2004-09-27 23:09 UTC (permalink / raw)
To: J.A. Magallon; +Cc: Linux Kernel list
On Mon, 2004-09-27 at 18:00, J.A. Magallon wrote:
> Just a 'me-too', with a slightly different oops:
Can you try the following patch please?
--
Paul Fulghum
paulkf@microgate.com
--- a/drivers/net/e100.c 2004-09-27 09:57:35.000000000 -0500
+++ b/drivers/net/e100.c 2004-09-27 16:00:12.115482112 -0500
@@ -1675,9 +1675,6 @@
if((err = e100_rx_alloc_list(nic)))
return err;
-
- disable_irq(nic->pdev->irq);
-
if((err = e100_alloc_cbs(nic)))
goto err_rx_clean_list;
if((err = e100_hw_init(nic)))
@@ -1689,7 +1686,6 @@
nic->netdev->name, nic->netdev)))
goto err_no_irq;
e100_enable_irq(nic);
- enable_irq(nic->pdev->irq);
netif_wake_queue(nic->netdev);
return 0;
@@ -1700,7 +1696,6 @@
err_rx_clean_list:
e100_rx_clean_list(nic);
- enable_irq(nic->pdev->irq);
return err;
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
@ 2004-09-28 21:03 Feldman, Scott
2004-09-29 3:06 ` Paul Fulghum
0 siblings, 1 reply; 10+ messages in thread
From: Feldman, Scott @ 2004-09-28 21:03 UTC (permalink / raw)
To: Paul Fulghum, Venkatesan, Ganesh; +Cc: linux-kernel
> I suspect the correct thing is to remove
> disable_irq/enable_irq from e100_up.
> I don't see any purpose for these calls in e100_up.
I don't either! This doesn't look right to me at all.
These enable_irq/disable_irq calls got added recently to -mm, probably
in the fix-for-spurious-interrupts-on-e100-resume-2.patch. Maybe just
the disable_irq call is all that is needed to solve the spurious
interrupt case?
Ganesh, can we back out patch out of -mm and go back to the drawing
board on the original problem? This patch will cause problems.
-scott
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-27 23:09 ` Paul Fulghum
@ 2004-09-28 22:13 ` J.A. Magallon
0 siblings, 0 replies; 10+ messages in thread
From: J.A. Magallon @ 2004-09-28 22:13 UTC (permalink / raw)
To: linux-kernel
On 2004.09.28, Paul Fulghum wrote:
> On Mon, 2004-09-27 at 18:00, J.A. Magallon wrote:
> > Just a 'me-too', with a slightly different oops:
>
> Can you try the following patch please?
>
It has killed the messages, and I have not seen any side effects.
Network is working fine.
--
J.A. Magallon <jamagallon()able!es> \ Software is like sex:
werewolf!able!es \ It's better when it's free
Mandrakelinux release 10.1 (Community) for i586
Linux 2.6.9-rc2-mm4 (gcc 3.4.1 (Mandrakelinux (Alpha 3.4.1-3mdk)) #2
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-28 21:03 Feldman, Scott
@ 2004-09-29 3:06 ` Paul Fulghum
2004-09-29 12:13 ` Alan Cox
0 siblings, 1 reply; 10+ messages in thread
From: Paul Fulghum @ 2004-09-29 3:06 UTC (permalink / raw)
To: Feldman, Scott; +Cc: Venkatesan, Ganesh, Linux Kernel list
On Tue, 2004-09-28 at 16:03, Feldman, Scott wrote:
> Maybe just
> the disable_irq call is all that is needed to solve the spurious
> interrupt case?
That would not work either.
request_irq() clears the enable/disable depth
only when registering the first device on that
interrupt.
If a device is sharing an interrupt with e100
and calls request_irq() before e100, then the
disable_irq() requires a matching enable_irq().
If e100 is the only or first device on
that interrupt, then the enable_irq() causes
the warning because the depth has been reset.
It is interesting behavior for request_irq.
I don't know if it was planned that way,
or is an unexpected artifact.
It makes the effect of the disable_irq call
indeterminate (to the driver) if made
before registering with the interrupt.
--
Paul Fulghum
paulkf@microgate.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-29 3:06 ` Paul Fulghum
@ 2004-09-29 12:13 ` Alan Cox
2004-09-29 13:31 ` Russell King
0 siblings, 1 reply; 10+ messages in thread
From: Alan Cox @ 2004-09-29 12:13 UTC (permalink / raw)
To: Paul Fulghum
Cc: Feldman, Scott, Venkatesan, Ganesh, Linux Kernel Mailing List
On Mer, 2004-09-29 at 04:06, Paul Fulghum wrote:
> It is interesting behavior for request_irq.
> I don't know if it was planned that way,
> or is an unexpected artifact.
> It makes the effect of the disable_irq call
> indeterminate (to the driver) if made
> before registering with the interrupt.
Essentially the code believes that you cannot use
disable_irq() until you've requested it. You can
however in 2.6 call request_irq with local interrupts
disabled.
We have a fundamental API design problem going back to
day one. The API IMHO should really be
struct irq *irq;
irq = allocate_irq(5, ...)
enable_irq(irq);
That would fix
- Drivers failing to load/init under freak low memory situations
- How to cleanly report irqs (because each irq can now have ->name)
- How to tell which shared irq users are disabled/enabled for the irq
poll/recovery code I posted (and is testing in -mm).
Unfortunately it would require changes to rather a lot of code.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-29 12:13 ` Alan Cox
@ 2004-09-29 13:31 ` Russell King
0 siblings, 0 replies; 10+ messages in thread
From: Russell King @ 2004-09-29 13:31 UTC (permalink / raw)
To: Alan Cox
Cc: Paul Fulghum, Feldman, Scott, Venkatesan, Ganesh,
Linux Kernel Mailing List
On Wed, Sep 29, 2004 at 01:13:26PM +0100, Alan Cox wrote:
> We have a fundamental API design problem going back to
> day one. The API IMHO should really be
>
> struct irq *irq;
> irq = allocate_irq(5, ...)
> enable_irq(irq);
>
> That would fix
> - Drivers failing to load/init under freak low memory situations
> - How to cleanly report irqs (because each irq can now have ->name)
> - How to tell which shared irq users are disabled/enabled for the irq
> poll/recovery code I posted (and is testing in -mm).
>
> Unfortunately it would require changes to rather a lot of code.
I suggested something like this a while back on the linux-arch list
but it didn't particularly have a good reception from the other
arch maintainers. If there's interest in it, I could dig it out.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 2.6.9-rc2-mm4 e100 enable_irq unbalanced from
2004-09-27 21:12 ` Paul Fulghum
@ 2004-09-30 18:21 ` Jeremy Fitzhardinge
0 siblings, 0 replies; 10+ messages in thread
From: Jeremy Fitzhardinge @ 2004-09-30 18:21 UTC (permalink / raw)
To: Paul Fulghum; +Cc: scott.feldman, linux-kernel
On Mon, 2004-09-27 at 16:12 -0500, Paul Fulghum wrote:
> On Mon, 2004-09-27 at 14:24, Paul Fulghum wrote:
> > The e100 module is generating a warning:
> >
> > Sep 27 13:30:29 deimos kernel: e100: Intel(R) PRO/100 Network Driver, 3.1.4-NAPI
> > Sep 27 13:30:29 deimos kernel: e100: Copyright(c) 1999-2004 Intel Corporation
> > Sep 27 13:30:29 deimos kernel: e100: eth0: e100_probe: addr 0xfecfc000, irq 16, MAC addr 00:90:27:3A:C5:E3
> > Sep 27 13:30:29 deimos kernel: enable_irq(16) unbalanced from ec83ff33
> > Sep 27 13:30:29 deimos kernel: [<c010923f>] enable_irq+0xcf/0xe0
> > Sep 27 13:30:29 deimos kernel: [<ec83ff33>] e100_up+0xf3/0x1f0 [e100]
>
> The following patch works for me and removes the warning.
>
> The disable_irq/enable_irq is not needed because
> the ISR can't be called before calling request_irq,
> the hardware is initialized before calling request_irq,
> and request_irq itself enables the interrupt if needed.
>
> Comments?
Hi,
That change was my fault, to address a problem with doing a resume from
suspend. The e100 in my laptop seems to emit a spurious interrupt on
resume, and the intention was to block interrupts until the handler had
been installed - otherwise I get a "got interrupt X and nobody cared"
message.
While this works for me (probably because IRQ11 is heavily shared), it's
apparently pretty bogus. But it did achieve the goal of getting the
problem looked at...
J
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2004-09-30 18:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-27 19:24 2.6.9-rc2-mm4 e100 enable_irq unbalanced from Paul Fulghum
2004-09-27 21:12 ` Paul Fulghum
2004-09-30 18:21 ` Jeremy Fitzhardinge
2004-09-27 23:00 ` J.A. Magallon
2004-09-27 23:09 ` Paul Fulghum
2004-09-28 22:13 ` J.A. Magallon
-- strict thread matches above, loose matches on Subject: below --
2004-09-28 21:03 Feldman, Scott
2004-09-29 3:06 ` Paul Fulghum
2004-09-29 12:13 ` Alan Cox
2004-09-29 13:31 ` Russell King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox