2008-03-04 06:53:35

by Greg Ungerer

Subject: [M68KNOMMU]: fix fec driver interrupt races

The FEC driver has a common interrupt handler for all interrupt event
types. It is raised on a number of distinct interrupt vectors.
This handler can't be re-entered while processing an interrupt, so
make sure all requested vectors are flagged as IRQF_DISABLED.

Signed-off-by: Greg Ungerer <[email protected]>
---

diff -Naurp linux-2.6.25-rc3/drivers/net/fec.c linux-2.6.25-rc3.foo/drivers/net/fec.c
--- linux-2.6.25-rc3/drivers/net/fec.c 2008-01-25 08:58:37.000000000 +1000
+++ linux-2.6.25-rc3.foo/drivers/net/fec.c 2008-03-04 16:38:31.000000000 +1000
@@ -1253,7 +1253,7 @@ static void __inline__ fec_request_intrs

/* Setup interrupt handlers. */
for (idp = id; idp->name; idp++) {
- if (request_irq(idp->irq, idp->handler, 0, idp->name, dev) != 0)
+ if (request_irq(idp->irq, idp->handler, IRQF_DISABLED, idp->name, dev) != 0)
printk("FEC: Could not allocate %s IRQ(%d)!\n", idp->name, idp->irq);
}

@@ -1382,7 +1382,7 @@ static void __inline__ fec_request_intrs

/* Setup interrupt handlers. */
for (idp = id; idp->name; idp++) {
- if (request_irq(b+idp->irq, fec_enet_interrupt, 0, idp->name, dev) != 0)
+ if (request_irq(b+idp->irq, fec_enet_interrupt, IRQF_DISABLED, idp->name, dev) != 0)
printk("FEC: Could not allocate %s IRQ(%d)!\n", idp->name, b+idp->irq);
}

@@ -1553,7 +1553,7 @@ static void __inline__ fec_request_intrs

/* Setup interrupt handlers. */
for (idp = id; idp->name; idp++) {
- if (request_irq(b+idp->irq,fec_enet_interrupt,0,idp->name,dev)!=0)
+ if (request_irq(b+idp->irq, fec_enet_interrupt, IRQF_DISABLED, idp->name,dev) != 0)
printk("FEC: Could not allocate %s IRQ(%d)!\n", idp->name, b+idp->irq);
}

@@ -1680,7 +1680,7 @@ static void __inline__ fec_request_intrs

/* Setup interrupt handlers. */
for (idp = id; idp->name; idp++) {
- if (request_irq(b+idp->irq,fec_enet_interrupt,0,idp->name,dev)!=0)
+ if (request_irq(b+idp->irq, fec_enet_interrupt, IRQF_DISABLED, idp->name,dev) != 0)
printk("FEC: Could not allocate %s IRQ(%d)!\n",
idp->name, b+idp->irq);
}

2008-03-05 11:34:16

by Jeff Garzik

Subject: Re: [M68KNOMMU]: fix fec driver interrupt races

Greg Ungerer wrote:
> The FEC driver has a common interrupt handler for all interrupt event
> types. It is raised on a number of distinct interrupt vectors.
> This handler can't be re-entered while processing an interrupt, so
> make sure all requested vectors are flagged as IRQF_DISABLED.
>
> Signed-off-by: Greg Ungerer <[email protected]>

NAK -- add spinlocks to your interrupt handling like you are supposed to

2008-03-05 23:01:23

by Greg Ungerer

Subject: Re: [M68KNOMMU]: fix fec driver interrupt races

Hi Jeff,

Jeff Garzik wrote:
> Greg Ungerer wrote:
>> The FEC driver has a common interrupt handler for all interrupt event
>> types. It is raised on a number of distinct interrupt vectors.
>> This handler can't be re-entered while processing an interrupt, so
>> make sure all requested vectors are flagged as IRQF_DISABLED.
>>
>> Signed-off-by: Greg Ungerer <[email protected]>
>
> NAK -- add spinlocks to your interrupt handling like you are supposed to

A better fix is to use the separate vectors, avoiding spinlocks
in most cases. But this is a simple immediate fix for 2.6.25.
The better solution is being worked on, but it needs some
reasonable testing before it is ready for inclusion, and that
won't be till post 2.6.25.

(This is a regression, fixes in the m68knommu interrupt subsystem
exposed this).

Regards
Greg



------------------------------------------------------------------------
Greg Ungerer -- Chief Software Dude EMAIL: [email protected]
Secure Computing Corporation PHONE: +61 7 3435 2888
825 Stanley St, FAX: +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia WEB: http://www.SnapGear.com