FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian Kernel

 
 
LinkBack Thread Tools
 
Old 04-04-2010, 04:33 PM
Ben Hutchings
 
Default Bug#566522: 3c503: Fix IRQ probing

The driver attempts to select an IRQ for the NIC automatically by
testing which of the supported IRQs are available and then probing
each available IRQ with probe_irq_{on,off}(). There are obvious race
conditions here, besides which:
1. The test for availability is done by passing a NULL handler, which
now always returns -EINVAL, thus the device cannot be opened:
<http://bugs.debian.org/566522>
2. probe_irq_off() will report only the first ISA IRQ handled,
potentially leading to a false negative.

There was another bug that meant it ignored all error codes from
request_irq() except -EBUSY, so it would 'succeed' despite this
(possibly causing conflicts with other ISA devices). This was fixed
by ab08999d6029bb2c79c16be5405d63d2bedbdfea 'WARNING: some
request_irq() failures ignored in el2_open()', which exposed bug 1.

This patch:
1. Replaces the use of probe_irq_{on,off}() with a real interrupt handler
2. Adds a delay before checking the interrupt-seen flag
3. Disables interrupts on all failure paths
4. Distinguishes error codes from the second request_irq() call,
consistently with the first

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
drivers/net/3c503.c | 42 ++++++++++++++++++++++++++++++------------
1 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/net/3c503.c b/drivers/net/3c503.c
index 66e0323..b74a0ea 100644
--- a/drivers/net/3c503.c
+++ b/drivers/net/3c503.c
@@ -380,6 +380,12 @@ out:
return retval;
}

+static irqreturn_t el2_probe_interrupt(int irq, void *seen)
+{
+ *(bool *)seen = true;
+ return IRQ_HANDLED;
+}
+
static int
el2_open(struct net_device *dev)
{
@@ -391,23 +397,35 @@ el2_open(struct net_device *dev)

outb(EGACFR_NORM, E33G_GACFR); /* Enable RAM and interrupts. */
do {
- retval = request_irq(*irqp, NULL, 0, "bogus", dev);
- if (retval >= 0) {
+ bool seen;
+
+ retval = request_irq(*irqp, el2_probe_interrupt, 0,
+ dev->name, &seen);
+ if (retval == -EBUSY)
+ continue;
+ if (retval < 0)
+ goto err_disable;
+
/* Twinkle the interrupt, and check if it's seen. */
- unsigned long cookie = probe_irq_on();
+ seen = false;
+ smp_wmb();
outb_p(0x04 << ((*irqp == 9) ? 2 : *irqp), E33G_IDCFR);
outb_p(0x00, E33G_IDCFR);
- if (*irqp == probe_irq_off(cookie) && /* It's a good IRQ line! */
- ((retval = request_irq(dev->irq = *irqp,
- eip_interrupt, 0,
- dev->name, dev)) == 0))
- break;
- } else {
- if (retval != -EBUSY)
- return retval;
- }
+ msleep(1);
+ free_irq(*irqp, el2_probe_interrupt);
+ if (!seen)
+ continue;
+
+ retval = request_irq(dev->irq = *irqp, eip_interrupt, 0,
+ dev->name, dev);
+ if (retval == -EBUSY)
+ continue;
+ if (retval < 0)
+ goto err_disable;
} while (*++irqp);
+
if (*irqp == 0) {
+ err_disable:
outb(EGACFR_IRQOFF, E33G_GACFR); /* disable interrupts. */
return -EAGAIN;
}
--
1.7.0.3





--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 1270398809.8341.47.camel@localhost">http://lists.debian.org/1270398809.8341.47.camel@localhost
 
Old 04-08-2010, 03:56 AM
David Miller
 
Default Bug#566522: 3c503: Fix IRQ probing

From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 04 Apr 2010 17:33:29 +0100

> The driver attempts to select an IRQ for the NIC automatically by
> testing which of the supported IRQs are available and then probing
> each available IRQ with probe_irq_{on,off}(). There are obvious race
> conditions here, besides which:
> 1. The test for availability is done by passing a NULL handler, which
> now always returns -EINVAL, thus the device cannot be opened:
> <http://bugs.debian.org/566522>
> 2. probe_irq_off() will report only the first ISA IRQ handled,
> potentially leading to a false negative.
>
> There was another bug that meant it ignored all error codes from
> request_irq() except -EBUSY, so it would 'succeed' despite this
> (possibly causing conflicts with other ISA devices). This was fixed
> by ab08999d6029bb2c79c16be5405d63d2bedbdfea 'WARNING: some
> request_irq() failures ignored in el2_open()', which exposed bug 1.
>
> This patch:
> 1. Replaces the use of probe_irq_{on,off}() with a real interrupt handler
> 2. Adds a delay before checking the interrupt-seen flag
> 3. Disables interrupts on all failure paths
> 4. Distinguishes error codes from the second request_irq() call,
> consistently with the first
>
> Compile-tested only.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

This looks logically fine, but I'm tossing this into net-next-2.6
because of the limited tester space for this driver.

Thanks Ben.



--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20100407.205618.201234628.davem@davemloft.net">htt p://lists.debian.org/20100407.205618.201234628.davem@davemloft.net
 

Thread Tools




All times are GMT. The time now is 09:12 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org