* Re: 2.5.63: 'Debug: sleeping function called from illegal context
@ 2003-03-02 10:23 Manfred Spraul
2003-03-02 21:54 ` Alan Cox
0 siblings, 1 reply; 5+ messages in thread
From: Manfred Spraul @ 2003-03-02 10:23 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel, Dr. David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 914 bytes --]
Alan wrote:
>On Sat, 2003-03-01 at 21:05, Dr. David Alan Gilbert wrote:
>> Hi,
>> 2.5.63 had a good go at trying to boot for me; the only error during
>> boot was 'Debug: sleeping function called from illegal context at
>> mm/slab.c:1617' during the IDE startup.
>
>Known problem. Its a bug in the request_irq code on x86. IDE just
>happens to be a victim of it.
>
>
I would call it an IDE bug: request_irq is not atomic, there are dozends
of kmalloc(GFP_KERNEL) calls in the implementation.
And on some archs it might be impossible to implement it without
sleeping - e.g. on a NUMA arch an IPI to the right node could be necessary.
What about fixing ide? If ide can't handle early interrupts, then it can use
disable_irq();
request_irq();
spin_lock_irq(&ide_lock);
do_setup();
spin_unlock_irq(&ide_lock);
enable_irq();
I've attached a proposal - what do you think?
--
Manfred
[-- Attachment #2: patch-ide --]
[-- Type: text/plain, Size: 5354 bytes --]
// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 5
// SUBLEVEL = 63
// EXTRAVERSION =
--- 2.5/include/linux/ide.h 2003-02-26 19:08:43.000000000 +0100
+++ build-2.5/include/linux/ide.h 2003-03-02 10:51:19.000000000 +0100
@@ -1730,6 +1730,19 @@
extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
extern spinlock_t ide_lock;
+extern struct semaphore ide_cfg_sem;
+/*
+ * Structure locking:
+ *
+ * ide_cfg_sem and ide_lock together protect changes to
+ * ide_hwif_t->{next,hwgroup}
+ * ide_drive_t->next
+ *
+ * ide_hwgroup_t->busy: ide_lock
+ * ide_hwgroup_t->hwif: ide_lock
+ * ide_hwif_t->mate: constant, no locking
+ * ide_drive_t->hwif: constant, no locking
+ */
#define local_irq_set(flags) do { local_save_flags((flags)); local_irq_enable(); } while (0)
--- 2.5/drivers/ide/ide-probe.c 2003-02-26 19:08:29.000000000 +0100
+++ build-2.5/drivers/ide/ide-probe.c 2003-03-02 11:15:40.000000000 +0100
@@ -1032,16 +1032,17 @@
*/
static int init_irq (ide_hwif_t *hwif)
{
- unsigned long flags;
unsigned int index;
+ int do_enable_irq;
ide_hwgroup_t *hwgroup, *new_hwgroup;
ide_hwif_t *match = NULL;
/* Allocate the buffer and potentially sleep first */
new_hwgroup = kmalloc(sizeof(ide_hwgroup_t),GFP_KERNEL);
-
- spin_lock_irqsave(&ide_lock, flags);
+ BUG_ON(in_interrupt());
+ BUG_ON(irqs_disabled());
+ down(&ide_cfg_sem);
hwif->hwgroup = NULL;
#if MAX_HWIFS > 1
/*
@@ -1078,7 +1079,7 @@
} else {
hwgroup = new_hwgroup;
if (!hwgroup) {
- spin_unlock_irqrestore(&ide_lock, flags);
+ up(&ide_cfg_sem);
return 1;
}
memset(hwgroup, 0, sizeof(ide_hwgroup_t));
@@ -1095,6 +1096,7 @@
/*
* Allocate the irq, if not already obtained for another hwif
*/
+ do_enable_irq = 0;
if (!match || match->irq != hwif->irq) {
int sa = SA_INTERRUPT;
#if defined(__mc68000__) || defined(CONFIG_APUS)
@@ -1112,17 +1114,23 @@
/* clear nIEN */
hwif->OUTB(0x08, hwif->io_ports[IDE_CONTROL_OFFSET]);
+ disable_irq(hwif->irq);
if (request_irq(hwif->irq,&ide_intr,sa,hwif->name,hwgroup)) {
if (!match)
kfree(hwgroup);
- spin_unlock_irqrestore(&ide_lock, flags);
+ enable_irq(&hwif->irq);
+ up(&ide_cfg_sem);
return 1;
}
+ do_enable_irq = 1;
}
/*
* Everything is okay, so link us into the hwgroup
*/
+ spin_lock_irq(&ide_lock);
+ if(do_enable_irq)
+ enable_irq(hwif->irq);
hwif->hwgroup = hwgroup;
hwif->next = hwgroup->hwif->next;
hwgroup->hwif->next = hwif;
@@ -1135,9 +1143,9 @@
hwgroup->drive = drive;
drive->next = hwgroup->drive->next;
hwgroup->drive->next = drive;
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irq(&ide_lock);
ide_init_queue(drive);
- spin_lock_irqsave(&ide_lock, flags);
+ spin_lock_irq(&ide_lock);
}
if (!hwgroup->hwif) {
@@ -1148,7 +1156,7 @@
}
/* all CPUs; safe now that hwif->hwgroup is set up */
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irq(&ide_lock);
#if !defined(__mc68000__) && !defined(CONFIG_APUS) && !defined(__sparc__)
printk("%s at 0x%03lx-0x%03lx,0x%03lx on irq %d", hwif->name,
@@ -1168,6 +1176,7 @@
printk(" (%sed with %s)",
hwif->sharing_irq ? "shar" : "serializ", match->name);
printk("\n");
+ up(&ide_cfg_sem);
return 0;
}
--- 2.5/drivers/ide/ide.c 2003-02-26 19:08:29.000000000 +0100
+++ build-2.5/drivers/ide/ide.c 2003-03-02 11:04:16.000000000 +0100
@@ -177,6 +177,7 @@
static int system_bus_speed; /* holds what we think is VESA/PCI bus speed */
static int initializing; /* set while initializing built-in drivers */
+DECLARE_MUTEX(ide_cfg_sem);
spinlock_t ide_lock __cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
static int ide_scan_direction; /* THIS was formerly 2.2.x pci=reverse */
@@ -584,13 +585,15 @@
ide_hwif_t *hwif, *g;
ide_hwgroup_t *hwgroup;
int irq_count = 0, unit, i;
- unsigned long flags;
ide_hwif_t old_hwif;
if (index >= MAX_HWIFS)
BUG();
- spin_lock_irqsave(&ide_lock, flags);
+ BUG_ON(in_interrupt());
+ BUG_ON(irqs_disabled());
+ down(&ide_cfg_sem);
+ spin_lock_irq(&ide_lock);
hwif = &ide_hwifs[index];
if (!hwif->present)
goto abort;
@@ -605,7 +608,7 @@
}
hwif->present = 0;
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irq(&ide_lock);
for (unit = 0; unit < MAX_DRIVES; ++unit) {
drive = &hwif->drives[unit];
@@ -618,7 +621,6 @@
#ifdef CONFIG_PROC_FS
destroy_proc_ide_drives(hwif);
#endif
- spin_lock_irqsave(&ide_lock, flags);
hwgroup = hwif->hwgroup;
/*
@@ -633,6 +635,7 @@
if (irq_count == 1)
free_irq(hwif->irq, hwgroup);
+ spin_lock_irq(&ide_lock);
/*
* Note that we only release the standard ports,
* and do not even try to handle any extra ports
@@ -813,7 +816,8 @@
hwif->hwif_data = old_hwif.hwif_data;
abort:
- spin_unlock_irqrestore(&ide_lock, flags);
+ spin_unlock_irq(&ide_lock);
+ up(&ide_cfg_sem);
}
EXPORT_SYMBOL(ide_unregister);
--- 2.5/drivers/ide/ide-io.c 2003-01-03 22:37:31.000000000 +0100
+++ build-2.5/drivers/ide/ide-io.c 2003-03-02 10:32:48.000000000 +0100
@@ -745,8 +745,8 @@
/* for atari only: POSSIBLY BROKEN HERE(?) */
ide_get_lock(ide_intr, hwgroup);
- /* necessary paranoia: ensure IRQs are masked on local CPU */
- local_irq_disable();
+ /* caller must own ide_lock */
+ BUG_ON(!irqs_disabled());
while (!hwgroup->busy) {
hwgroup->busy = 1;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 2.5.63: 'Debug: sleeping function called from illegal context
@ 2003-03-02 15:15 Manfred Spraul
2003-03-02 19:13 ` Russell King
2003-03-02 22:37 ` Alan Cox
0 siblings, 2 replies; 5+ messages in thread
From: Manfred Spraul @ 2003-03-02 15:15 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-kernel, Benjamin Herrenschmidt
[-- Attachment #1: Type: text/plain, Size: 400 bytes --]
Alan wrote:
>Unfortunately ten years ago someone created 'register_and_activate_irq'
>calling it 'register_irq', and it hasn't yet been fixed.
>
>
Then it's time to fix that.
I would propose something like the attached patch - it handles all archs
that support disable_irq on an unregistered interrupt. The remaining
arch [which one, btw] must implement request_irq_disabled().
--
Manfred
[-- Attachment #2: patch-rqd --]
[-- Type: text/plain, Size: 2994 bytes --]
// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 5
// SUBLEVEL = 63
// EXTRAVERSION =
--- 2.5/include/linux/interrupt.h 2002-11-26 17:25:18.000000000 +0100
+++ build-2.5/include/linux/interrupt.h 2003-03-02 15:48:40.000000000 +0100
@@ -23,6 +23,9 @@
extern int request_irq(unsigned int,
void (*handler)(int, void *, struct pt_regs *),
unsigned long, const char *, void *);
+extern int request_irq_disabled(unsigned int,
+ void (*handler)(int, void *, struct pt_regs *),
+ unsigned long, const char *, void *);
extern void free_irq(unsigned int, void *);
/*
--- 2.5/kernel/ksyms.c 2003-02-26 19:08:44.000000000 +0100
+++ build-2.5/kernel/ksyms.c 2003-03-02 15:54:06.000000000 +0100
@@ -404,6 +404,7 @@
EXPORT_SYMBOL(add_timer);
EXPORT_SYMBOL(del_timer);
EXPORT_SYMBOL(request_irq);
+EXPORT_SYMBOL(request_irq_disabled);
EXPORT_SYMBOL(free_irq);
EXPORT_SYMBOL(irq_stat);
diff -urN --exclude='*.cmd' --exclude crc32table.h --exclude '*.orig' 2.5/lib/irq.c build-2.5/lib/irq.c
--- 2.5/lib/irq.c 1970-01-01 01:00:00.000000000 +0100
+++ build-2.5/lib/irq.c 2003-03-02 16:10:58.000000000 +0100
@@ -0,0 +1,39 @@
+/*
+ * linux/lib/irq.c: arch independant helper functions for irq handling.
+ *
+ * Copyright (C) 2003 Manfred Spraul
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/config.h>
+#include <linux/interrupt.h>
+
+#ifndef __HAVE_ARCH_REQUEST_IRQ_DISABLED
+int request_irq_disabled(unsigned int irq,
+ void (*handler)(int, void *, struct pt_regs *),
+ unsigned long irqflags,
+ const char * devname,
+ void *dev_id)
+{
+ int retval;
+ disable_irq(irq);
+ retval = request_irq(irq, handler, irqflags, devname, dev_id);
+ if (retval < 0)
+ enable_irq(irq);
+ return retval;
+}
+#endif
diff -urN --exclude='*.cmd' --exclude crc32table.h --exclude '*.orig' 2.5/lib/Makefile build-2.5/lib/Makefile
--- 2.5/lib/Makefile 2003-02-26 19:08:44.000000000 +0100
+++ build-2.5/lib/Makefile 2003-03-02 15:54:30.000000000 +0100
@@ -10,7 +10,7 @@
obj-y := errno.o ctype.o string.o vsprintf.o brlock.o cmdline.o \
bust_spinlocks.o rbtree.o radix-tree.o dump_stack.o \
- kobject.o idr.o
+ kobject.o idr.o irq.o
obj-$(CONFIG_RWSEM_GENERIC_SPINLOCK) += rwsem-spinlock.o
obj-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 2.5.63: 'Debug: sleeping function called from illegal context
2003-03-02 15:15 Manfred Spraul
@ 2003-03-02 19:13 ` Russell King
2003-03-02 22:37 ` Alan Cox
1 sibling, 0 replies; 5+ messages in thread
From: Russell King @ 2003-03-02 19:13 UTC (permalink / raw)
To: Manfred Spraul; +Cc: Alan Cox, linux-kernel, Benjamin Herrenschmidt
On Sun, Mar 02, 2003 at 04:15:25PM +0100, Manfred Spraul wrote:
> I would propose something like the attached patch - it handles all archs
> that support disable_irq on an unregistered interrupt. The remaining
> arch [which one, btw] must implement request_irq_disabled().
>...
> +{
> + int retval;
> + disable_irq(irq);
> + retval = request_irq(irq, handler, irqflags, devname, dev_id);
> + if (retval < 0)
> + enable_irq(irq);
> + return retval;
> +}
> +#endif
request_irq() explicitly enables the interrupt source no matter how many
times you call disable_irq() before hand.
--
Russell King (rmk@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 2.5.63: 'Debug: sleeping function called from illegal context
2003-03-02 10:23 2.5.63: 'Debug: sleeping function called from illegal context Manfred Spraul
@ 2003-03-02 21:54 ` Alan Cox
0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2003-03-02 21:54 UTC (permalink / raw)
To: Manfred Spraul; +Cc: Linux Kernel Mailing List, Dr. David Alan Gilbert
On Sun, 2003-03-02 at 10:23, Manfred Spraul wrote:
> sleeping - e.g. on a NUMA arch an IPI to the right node could be necessary.
> What about fixing ide? If ide can't handle early interrupts, then it can use
>
> disable_irq();
disable_irq(n) isn't something all platforms seem to define before the
IRQ is requested, In addition disable_irq() is not supported on some
systems the IDE code supports. I have to have local CPU interrupts
disabled on the CPU at the point the IRQ becomes live.
The former we could probably solve with "SA_DISABLE" to request an IRQ
that is disabled, but I'm not sure how we deal with platforms which
plain and simply do not have 'mask irq' functionality but are using a
level triggered interrupt.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 2.5.63: 'Debug: sleeping function called from illegal context
2003-03-02 15:15 Manfred Spraul
2003-03-02 19:13 ` Russell King
@ 2003-03-02 22:37 ` Alan Cox
1 sibling, 0 replies; 5+ messages in thread
From: Alan Cox @ 2003-03-02 22:37 UTC (permalink / raw)
To: Manfred Spraul; +Cc: Linux Kernel Mailing List, Benjamin Herrenschmidt
On Sun, 2003-03-02 at 15:15, Manfred Spraul wrote:
> I would propose something like the attached patch - it handles all archs
> that support disable_irq on an unregistered interrupt. The remaining
> arch [which one, btw] must implement request_irq_disabled().
Some m68k at least and possibly others.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-03-02 21:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-03-02 10:23 2.5.63: 'Debug: sleeping function called from illegal context Manfred Spraul
2003-03-02 21:54 ` Alan Cox
-- strict thread matches above, loose matches on Subject: below --
2003-03-02 15:15 Manfred Spraul
2003-03-02 19:13 ` Russell King
2003-03-02 22:37 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox