* [patch] warn on release_region() from irq context
@ 2006-01-28 14:43 Ingo Molnar
2006-01-28 15:00 ` Ingo Molnar
0 siblings, 1 reply; 4+ messages in thread
From: Ingo Molnar @ 2006-01-28 14:43 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
it is not legal to call release_region() from hardirq/softirq context.
Add a WARN_ON() so that we find such cases.
Signed-off-by: Ingo Molnar <mingo@elte.hu>
----
kernel/resource.c | 3 +++
1 files changed, 3 insertions(+)
Index: linux/kernel/resource.c
===================================================================
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -18,6 +18,7 @@
#include <linux/fs.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/interrupt.h>
#include <asm/io.h>
@@ -486,6 +487,8 @@ void __release_region(struct resource *p
struct resource **p;
unsigned long end;
+ WARN_ON(in_interrupt());
+
p = &parent->child;
end = start + n - 1;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] warn on release_region() from irq context
2006-01-28 14:43 [patch] warn on release_region() from irq context Ingo Molnar
@ 2006-01-28 15:00 ` Ingo Molnar
2006-01-28 16:02 ` Ingo Molnar
2006-01-29 13:06 ` Ingo Molnar
0 siblings, 2 replies; 4+ messages in thread
From: Ingo Molnar @ 2006-01-28 15:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Arjan van de Ven
the rationale for this patch is that the lock validator (which now
tracks rwlock dependencies too, see output below) noticed that floppy.c
does a release_region() from softirq context -> ouch. I'm working on the
floppy.c fix for that.
Ingo
============================
[ BUG: illegal lock usage! ]
----------------------------
illegal {softirq-on} -> {in-softirq} usage.
rcu_torture_rea/264 [HC0[0]:SC1[2]:HE1:SE0] takes:
(resource_lock){+-}, at: [<c0123997>] __release_region+0x37/0xf0
{softirq-on} state was registered at:
[<c0122c85>] irq_exit+0x45/0x50
hardirqs last enabled at: [<c011d39e>] vprintk+0x32e/0x3e0
softirqs last enabled at: [<c0122c85>] irq_exit+0x45/0x50
other info that might help us debug this:
no locks held by rcu_torture_rea/264.
stack backtrace:
[<c010432d>] show_trace+0xd/0x10
[<c0104347>] dump_stack+0x17/0x20
[<c0139078>] print_usage_bug+0x1d8/0x230
[<c0139436>] mark_lock+0x366/0x3a0
[<c0139903>] debug_lock_chain+0x493/0x1090
[<c013a53d>] debug_lock_chain_spin+0x3d/0x60
[<c0268522>] _raw_write_lock+0x32/0x1a0
[<c04dc158>] _write_lock+0x8/0x10
[<c0123997>] __release_region+0x37/0xf0
[<c02f5010>] floppy_release_irq_and_dma+0x140/0x200
[<c02f3577>] set_dor+0xc7/0x1b0
[<c02f65e1>] motor_off_callback+0x21/0x30
[<c01276c5>] run_timer_softirq+0xf5/0x1f0
[<c0122fc7>] __do_softirq+0x97/0x130
[<c01054a9>] do_softirq+0x69/0x100
=======================
[<c0122c85>] irq_exit+0x45/0x50
[<c010f504>] smp_apic_timer_interrupt+0x54/0x60
[<c010393b>] apic_timer_interrupt+0x27/0x2c
[<c0266b79>] __delay+0x9/0x10
[<c0266be1>] __udelay+0x31/0x40
[<c01445e3>] rcu_torture_reader+0x83/0x140
[<c0131cca>] kthread+0xca/0xd0
[<c0100ef5>] kernel_thread_helper+0x5/0x10
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] warn on release_region() from irq context
2006-01-28 15:00 ` Ingo Molnar
@ 2006-01-28 16:02 ` Ingo Molnar
2006-01-29 13:06 ` Ingo Molnar
1 sibling, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2006-01-28 16:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Arjan van de Ven
* Ingo Molnar <mingo@elte.hu> wrote:
> the rationale for this patch is that the lock validator (which now
> tracks rwlock dependencies too, see output below) noticed that
> floppy.c does a release_region() from softirq context -> ouch. I'm
> working on the floppy.c fix for that.
ugh. floppy.c is a mess. I did the patch below - but that's not enough,
it does stupid things like request_region() and release_region() from
the timer irq - so floppy.c file needs a serious redesign to fix these
deadlocks :-(
Ingo
floppy.c does alot of irq-unsafe work within floppy_release_irq_and_dma():
free_irq(), release_region() ... so when executing in irq context, push
the whole function into keventd.
Signed-off-by: Ingo Molnar <mingo@elte.hu>
----
drivers/block/floppy.c | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
Index: linux/drivers/block/floppy.c
===================================================================
--- linux.orig/drivers/block/floppy.c
+++ linux/drivers/block/floppy.c
@@ -561,6 +561,21 @@ static int floppy_grab_irq_and_dma(void)
static void floppy_release_irq_and_dma(void);
/*
+ * Interrupt, DMA and region freeing must not be done from IRQ
+ * context - e.g. irq-unregistration means /proc VFS work, region
+ * release takes an irq-unsafe lock, etc. So we push this work
+ * into keventd:
+ */
+static void fd_release_fn(void *data)
+{
+ mutex_lock(&open_lock);
+ floppy_release_irq_and_dma();
+ mutex_unlock(&open_lock);
+}
+
+static DECLARE_WORK(floppy_release_irq_and_dma_work, fd_release_fn, NULL);
+
+/*
* The "reset" variable should be tested whenever an interrupt is scheduled,
* after the commands have been sent. This is to ensure that the driver doesn't
* get wedged when the interrupt doesn't come because of a failed command.
@@ -824,7 +839,7 @@ static int set_dor(int fdc, char mask, c
if (newdor & FLOPPY_MOTOR_MASK)
floppy_grab_irq_and_dma();
if (olddor & FLOPPY_MOTOR_MASK)
- floppy_release_irq_and_dma();
+ schedule_work(&floppy_release_irq_and_dma_work);
return olddor;
}
@@ -905,6 +920,8 @@ static int _lock_fdc(int drive, int inte
set_current_state(TASK_RUNNING);
remove_wait_queue(&fdc_wait, &wait);
+
+ flush_scheduled_work();
}
command_status = FD_COMMAND_NONE;
@@ -938,7 +955,7 @@ static inline void unlock_fdc(void)
if (elv_next_request(floppy_queue))
do_fd_request(floppy_queue);
spin_unlock_irqrestore(&floppy_lock, flags);
- floppy_release_irq_and_dma();
+ schedule_work(&floppy_release_irq_and_dma_work);
wake_up(&fdc_wait);
}
@@ -4628,6 +4645,12 @@ void cleanup_module(void)
del_timer_sync(&fd_timer);
blk_cleanup_queue(floppy_queue);
+ /*
+ * Wait for any asynchronous floppy_release_irq_and_dma()
+ * calls to finish first:
+ */
+ flush_scheduled_work();
+
if (usage_count)
floppy_release_irq_and_dma();
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] warn on release_region() from irq context
2006-01-28 15:00 ` Ingo Molnar
2006-01-28 16:02 ` Ingo Molnar
@ 2006-01-29 13:06 ` Ingo Molnar
1 sibling, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2006-01-29 13:06 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Arjan van de Ven
* Ingo Molnar <mingo@elte.hu> wrote:
> ============================
> [ BUG: illegal lock usage! ]
> ----------------------------
> illegal {softirq-on} -> {in-softirq} usage.
> rcu_torture_rea/264 [HC0[0]:SC1[2]:HE1:SE0] takes:
> (resource_lock){+-}, at: [<c0123997>] __release_region+0x37/0xf0
> {softirq-on} state was registered at:
> [<c0122c85>] irq_exit+0x45/0x50
> hardirqs last enabled at: [<c011d39e>] vprintk+0x32e/0x3e0
> softirqs last enabled at: [<c0122c85>] irq_exit+0x45/0x50
>
> other info that might help us debug this:
> no locks held by rcu_torture_rea/264.
>
> stack backtrace:
> [<c010432d>] show_trace+0xd/0x10
> [<c0104347>] dump_stack+0x17/0x20
> [<c0139078>] print_usage_bug+0x1d8/0x230
> [<c0139436>] mark_lock+0x366/0x3a0
> [<c0139903>] debug_lock_chain+0x493/0x1090
> [<c013a53d>] debug_lock_chain_spin+0x3d/0x60
> [<c0268522>] _raw_write_lock+0x32/0x1a0
> [<c04dc158>] _write_lock+0x8/0x10
> [<c0123997>] __release_region+0x37/0xf0
> [<c02f5010>] floppy_release_irq_and_dma+0x140/0x200
> [<c02f3577>] set_dor+0xc7/0x1b0
> [<c02f65e1>] motor_off_callback+0x21/0x30
> [<c01276c5>] run_timer_softirq+0xf5/0x1f0
> [<c0122fc7>] __do_softirq+0x97/0x130
> [<c01054a9>] do_softirq+0x69/0x100
btw., Arjan discovered that this bug is not theoretical, and the
deadlock scenario is experienced by users:
http://i15.photobucket.com/albums/a396/johnny_blue/fedora-devel/28012006026.jpg
Ingo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-01-29 13:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-28 14:43 [patch] warn on release_region() from irq context Ingo Molnar
2006-01-28 15:00 ` Ingo Molnar
2006-01-28 16:02 ` Ingo Molnar
2006-01-29 13:06 ` Ingo Molnar
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).