via-rhine: prevent oops when requesting an IRQ

PATCH In review Chuck Ebbert 2 participants 4 messages Nov 5, 2008 – Nov 10, 2008

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Chuck Ebbert

author

2008-11-05 00:24 UTC #0 lore raw

PATCH 1/1 via-rhine: prevent oops when requesting an IRQ

via-rhine: prevent oops when requesting an IRQ

via-rhine requests an IRQ before it's ready to handle an interrupt. It oopses when CONFIG_DEBUG_SHIRQ is enabled.

https://bugzilla.redhat.com/show_bug.cgi?id=469303

---

NOTE: UNTESTED: Is it okay to init the hardware before requesting the IRQ, or should that be done afterward?

drivers/net/via-rhine.c +14 ~~-11~~

Index: linux-2.6.27.noarch/drivers/net/via-rhine.c
===================================================================
@@ -1141,24 +1141,27 @@ static int rhine_open(struct net_device 
 	void __iomem *ioaddr = rp->base;
 	int rc;
 
-	rc = request_irq(rp->pdev->irq, &rhine_interrupt, IRQF_SHARED, dev->name,
-			dev);
+	rc = alloc_ring(dev);
 	if (rc)
 		return rc;
 
-	if (debug > 1)
-		printk(KERN_DEBUG "%s: rhine_open() irq %d.\n",
-		       dev->name, rp->pdev->irq);
-
-	rc = alloc_ring(dev);
-	if (rc) {
-		free_irq(rp->pdev->irq, dev);
-		return rc;
-	}
 	alloc_rbufs(dev);
 	alloc_tbufs(dev);
 	rhine_chip_reset(dev);
 	init_registers(dev);
+
+	rc = request_irq(rp->pdev->irq, &rhine_interrupt, IRQF_SHARED, dev->name,
+			dev);
+	if (rc) {
+		free_tbufs(dev);
+		free_rbufs(dev);
+		free_ring(dev);
+		return rc;
+	}
+
+	if (debug > 1)
+		printk(KERN_DEBUG "%s: rhine_open() irq %d.\n",
+		       dev->name, rp->pdev->irq);
 	if (debug > 2)
 		printk(KERN_DEBUG "%s: Done rhine_open(), status %4.4x "
 		       "MII status: %4.4x.\n",

Jeff Garzik

2008-11-06 06:07 UTC #1 lore raw

Quoted 11 lines · Chuck Ebbert

via-rhine: prevent oops when requesting an IRQ
via-rhine requests an IRQ before it's ready to handle an interrupt. It oopses when CONFIG_DEBUG_SHIRQ is enabled.
https://bugzilla.redhat.com/show_bug.cgi?id=469303
---
NOTE: UNTESTED: Is it okay to init the hardware before requesting the IRQ, or should that be done afterward?

First of all, _ideally_ your interrupt handler should be able to any state of software initialization, once registers are mapped. So I would first concentrate on "hardening" the interrupt handler, if feasible.

It is certainly a common technique to disable interrupts somehow, during initialization of the hardware. Note, though, that doing things before request_irq() is no real guarantee you are out of danger -- you might be on a shared PCI irq, and your init causes that interrupt to "scream" even though your driver has not registered the irq.

Jeff

Chuck Ebbert

author

2008-11-10 22:29 UTC #2 lore raw

Quoted 17 lines · On Thu, 06 Nov 2008 01:07:41 -0500 Jeff Garzik <[email protected]> wrote:

Quoted 11 lines · Chuck Ebbert
via-rhine: prevent oops when requesting an IRQ
via-rhine requests an IRQ before it's ready to handle an interrupt. It oopses when CONFIG_DEBUG_SHIRQ is enabled.
https://bugzilla.redhat.com/show_bug.cgi?id=469303
---
NOTE: UNTESTED: Is it okay to init the hardware before requesting the IRQ, or should that be done afterward?
First of all, _ideally_ your interrupt handler should be able to any state of software initialization, once registers are mapped. So I would first concentrate on "hardening" the interrupt handler, if feasible.

Quoted 6 lines

It is certainly a common technique to disable interrupts somehow, during initialization of the hardware. Note, though, that doing things before request_irq() is no real guarantee you are out of danger -- you might be on a shared PCI irq, and your init causes that interrupt to "scream" even though your driver has not registered the irq.

The problem with this driver is that it doesn't call alloc_ring() before request_irq() and then it oopses in the interrupt handler trying to access the ring.

Jeff Garzik

2008-11-10 22:35 UTC #3 lore raw

Quoted 15 lines · Chuck Ebbert

Quoted 12 lines · On Thu, 06 Nov 2008 01:07:41 -0500 Jeff Garzik <[email protected]> wrote:
Quoted 11 lines · Chuck Ebbert
via-rhine: prevent oops when requesting an IRQ
via-rhine requests an IRQ before it's ready to handle an interrupt. It oopses when CONFIG_DEBUG_SHIRQ is enabled.
https://bugzilla.redhat.com/show_bug.cgi?id=469303
---
NOTE: UNTESTED: Is it okay to init the hardware before requesting the IRQ, or should that be done afterward?

Quoted 8 lines

Quoted 4 lines
First of all, _ideally_ your interrupt handler should be able to any state of software initialization, once registers are mapped. So I would first concentrate on "hardening" the interrupt handler, if feasible.
It is certainly a common technique to disable interrupts somehow, during initialization of the hardware. Note, though, that doing things before request_irq() is no real guarantee you are out of danger -- you might be on a shared PCI irq, and your init causes that interrupt to "scream" even though your driver has not registered the irq.
The problem with this driver is that it doesn't call alloc_ring() before request_irq() and then it oopses in the interrupt handler trying to access the ring.

Allocation should definitely occur prior to request_irq()

Jeff