* [RFC+PATCH] calling request_irq() with lock held (+sungem fix)
@ 2003-07-30 17:30 Benjamin Herrenschmidt
2003-07-30 23:42 ` David S. Miller
0 siblings, 1 reply; 2+ messages in thread
From: Benjamin Herrenschmidt @ 2003-07-30 17:30 UTC (permalink / raw)
To: David S. Miller, Linus Torvalds; +Cc: linux-kernel mailing list
Hrm.. old problem: request_irq() called with a lock held.
This is unclear wether this should be safe or not, I now IDE used to
do that, but the current implementation of request_irq() on most archs
is definitely not safe to be called in a non-sleeping context.
i386 was sort-fixed by using GFP_ATOMIC in the kmalloc() done inside
request_irq() itself, but what about all of the proc related stuff
that gets done when setup_irq() calls register_irq_proc() ? So the
_fact_ is that the current implementations in archs, including i386,
are unsafe to call from "atomic" context.
David: this patch fixes sungem for that.
Cheers,
Ben.
diff -urN linux-2.5/drivers/net/sungem.c linuxppc-2.5-benh/drivers/net/sungem.c
--- linux-2.5/drivers/net/sungem.c 2003-07-29 08:50:59.000000000 -0400
+++ linuxppc-2.5-benh/drivers/net/sungem.c 2003-07-30 13:25:32.000000000 -0400
@@ -2101,17 +2101,14 @@
gp->hw_running = 1;
}
- spin_lock_irq(&gp->lock);
-
/* We can now request the interrupt as we know it's masked
* on the controller
*/
if (request_irq(gp->pdev->irq, gem_interrupt,
SA_SHIRQ, dev->name, (void *)dev)) {
- spin_unlock_irq(&gp->lock);
-
printk(KERN_ERR "%s: failed to request irq !\n", gp->dev->name);
+ spin_lock_irq(&gp->lock);
#ifdef CONFIG_PPC_PMAC
if (!hw_was_up && gp->pdev->vendor == PCI_VENDOR_ID_APPLE)
gem_apple_powerdown(gp);
@@ -2120,10 +2117,13 @@
gp->pm_timer.expires = jiffies + 10*HZ;
add_timer(&gp->pm_timer);
up(&gp->pm_sem);
+ spin_unlock_irq(&gp->lock);
return -EAGAIN;
}
+ spin_lock_irq(&gp->lock);
+
/* Allocate & setup ring buffers */
gem_init_rings(gp);
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC+PATCH] calling request_irq() with lock held (+sungem fix)
2003-07-30 17:30 [RFC+PATCH] calling request_irq() with lock held (+sungem fix) Benjamin Herrenschmidt
@ 2003-07-30 23:42 ` David S. Miller
0 siblings, 0 replies; 2+ messages in thread
From: David S. Miller @ 2003-07-30 23:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: torvalds, linux-kernel
On 30 Jul 2003 13:30:44 -0400
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> i386 was sort-fixed by using GFP_ATOMIC in the kmalloc() done inside
> request_irq() itself, but what about all of the proc related stuff
> that gets done when setup_irq() calls register_irq_proc() ? So the
> _fact_ is that the current implementations in archs, including i386,
> are unsafe to call from "atomic" context.
That's true.
> David: this patch fixes sungem for that.
Ok, I'll review this and probably apply it, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2003-07-30 23:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-30 17:30 [RFC+PATCH] calling request_irq() with lock held (+sungem fix) Benjamin Herrenschmidt
2003-07-30 23:42 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).