Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
When attempting to compile test11-pre2, I get the following compile
error.
arch/i386/mm/mm.o: In function `do_page_fault':
arch/i386/mm/mm.o(.text+0x781): undefined reference to `bust_spinlocks'
make: *** [vmlinux] Error 1
John Kacur
On Fri, 10 Nov 2000 00:32:49 -0500,
John Kacur <[email protected]> wrote:
>When attempting to compile test11-pre2, I get the following compile
>error.
>
>arch/i386/mm/mm.o: In function `do_page_fault':
>arch/i386/mm/mm.o(.text+0x781): undefined reference to `bust_spinlocks'
>make: *** [vmlinux] Error 1
Index: 0-test11-pre2.1/arch/i386/kernel/traps.c
--- 0-test11-pre2.1/arch/i386/kernel/traps.c Fri, 10 Nov 2000 13:10:37 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
+++ 0-test11-pre2.1(w)/arch/i386/kernel/traps.c Fri, 10 Nov 2000 15:56:54 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
@@ -382,6 +382,17 @@ static void unknown_nmi_error(unsigned c
printk("Do you have a strange power saving mode enabled?\n");
}
+/*
+ * Unlock any spinlocks which will prevent us from getting the
+ * message out (timerlist_lock is acquired through the
+ * console unblank code)
+ */
+void bust_spinlocks(void)
+{
+ spin_lock_init(&console_lock);
+ spin_lock_init(&timerlist_lock);
+}
+
#if CONFIG_X86_IO_APIC
int nmi_watchdog = 1;
@@ -396,17 +407,6 @@ __setup("nmi_watchdog=", setup_nmi_watch
extern spinlock_t console_lock, timerlist_lock;
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
-
-/*
- * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is aquired through the
- * console unblank code)
- */
-void bust_spinlocks(void)
-{
- spin_lock_init(&console_lock);
- spin_lock_init(&timerlist_lock);
-}
inline void nmi_watchdog_tick(struct pt_regs * regs)
{
Keith Owens wrote:
> Index: 0-test11-pre2.1/arch/i386/kernel/traps.c
> --- 0-test11-pre2.1/arch/i386/kernel/traps.c Fri, 10 Nov 2000 13:10:37 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
> +++ 0-test11-pre2.1(w)/arch/i386/kernel/traps.c Fri, 10 Nov 2000 15:56:54 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
> @@ -382,6 +382,17 @@ static void unknown_nmi_error(unsigned c
> printk("Do you have a strange power saving mode enabled?\n");
> }
>
> +/*
> + * Unlock any spinlocks which will prevent us from getting the
> + * message out (timerlist_lock is acquired through the
> + * console unblank code)
> + */
> +void bust_spinlocks(void)
> +{
> + spin_lock_init(&console_lock);
> + spin_lock_init(&timerlist_lock);
> +}
> +
> #if CONFIG_X86_IO_APIC
>
> int nmi_watchdog = 1;
> @@ -396,17 +407,6 @@ __setup("nmi_watchdog=", setup_nmi_watch
>
> extern spinlock_t console_lock, timerlist_lock;
> static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
Did you actually try this? You are moving bust_spinlocks above the
'extern spinlock_t' declarations intended for it.
My solution was to move the 'extern spinlock_t ...' also :)
Jeff
--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
On Fri, 10 Nov 2000 00:32:49 -0500,
John Kacur <[email protected]> wrote:
>When attempting to compile test11-pre2, I get the following compile
>error.
>
>arch/i386/mm/mm.o: In function `do_page_fault':
>arch/i386/mm/mm.o(.text+0x781): undefined reference to `bust_spinlocks'
>make: *** [vmlinux] Error 1
Oops, wrong patch.
Index: 0-test11-pre2.1/arch/i386/kernel/traps.c
--- 0-test11-pre2.1/arch/i386/kernel/traps.c Fri, 10 Nov 2000 13:10:37 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
+++ 0-test11-pre2.1(w)/arch/i386/kernel/traps.c Fri, 10 Nov 2000 16:06:48 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
@@ -382,6 +382,18 @@ static void unknown_nmi_error(unsigned c
printk("Do you have a strange power saving mode enabled?\n");
}
+extern spinlock_t console_lock, timerlist_lock;
+/*
+ * Unlock any spinlocks which will prevent us from getting the
+ * message out (timerlist_lock is acquired through the
+ * console unblank code)
+ */
+void bust_spinlocks(void)
+{
+ spin_lock_init(&console_lock);
+ spin_lock_init(&timerlist_lock);
+}
+
#if CONFIG_X86_IO_APIC
int nmi_watchdog = 1;
@@ -394,19 +406,7 @@ static int __init setup_nmi_watchdog(cha
__setup("nmi_watchdog=", setup_nmi_watchdog);
-extern spinlock_t console_lock, timerlist_lock;
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
-
-/*
- * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is aquired through the
- * console unblank code)
- */
-void bust_spinlocks(void)
-{
- spin_lock_init(&console_lock);
- spin_lock_init(&timerlist_lock);
-}
inline void nmi_watchdog_tick(struct pt_regs * regs)
{
John Kacur wrote:
>
> When attempting to compile test11-pre2, I get the following compile
> error.
>
> arch/i386/mm/mm.o: In function `do_page_fault':
> arch/i386/mm/mm.o(.text+0x781): undefined reference to `bust_spinlocks'
> make: *** [vmlinux] Error 1
It was inside an ifdef. Apologies.
This patch against test11-pre2 moves it to fault.c.
--- linux-2.4.0-test11-pre2/arch/i386/kernel/traps.c Fri Nov 10 15:59:15 2000
+++ linux/arch/i386/kernel/traps.c Fri Nov 10 15:52:40 2000
@@ -63,6 +63,7 @@
struct desc_struct idt_table[256] __attribute__((__section__(".data.idt"))) = { {0, 0}, };
extern int console_loglevel;
+extern void bust_spinlocks(void);
static inline void console_silent(void)
{
@@ -394,19 +395,7 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-extern spinlock_t console_lock, timerlist_lock;
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
-
-/*
- * Unlock any spinlocks which will prevent us from getting the
- * message out (timerlist_lock is aquired through the
- * console unblank code)
- */
-void bust_spinlocks(void)
-{
- spin_lock_init(&console_lock);
- spin_lock_init(&timerlist_lock);
-}
inline void nmi_watchdog_tick(struct pt_regs * regs)
{
--- linux-2.4.0-test11-pre2/arch/i386/mm/fault.c Fri Nov 10 15:59:15 2000
+++ linux/arch/i386/mm/fault.c Fri Nov 10 16:02:03 2000
@@ -24,7 +24,6 @@
#include <asm/hardirq.h>
extern void die(const char *,struct pt_regs *,long);
-extern void bust_spinlocks(void);
/*
* Ugly, ugly, but the goto's result in better assembly..
@@ -76,6 +75,19 @@
bad_area:
return 0;
+}
+
+extern spinlock_t console_lock, timerlist_lock;
+
+/*
+ * Unlock any spinlocks which will prevent us from getting the
+ * message out (timerlist_lock is aquired through the
+ * console unblank code)
+ */
+void bust_spinlocks(void)
+{
+ spin_lock_init(&console_lock);
+ spin_lock_init(&timerlist_lock);
}
asmlinkage void do_invalid_op(struct pt_regs *, unsigned long);
The notion of releasing a spin lock by initializing it seems IMHO, on
the face of it, way off. Firstly the protected area is no longer
protected which could lead to undefined errors/ crashes and secondly,
any future use of spinlocks to control preemption could have a lot of
trouble with this, principally because the locker is unknown.
In the case at hand, it would seem that an unlocked path to the console
is a more correct answer that gives the system a far better chance of
actually remaining viable.
George
Keith Owens wrote:
>
> On Fri, 10 Nov 2000 00:32:49 -0500,
> John Kacur <[email protected]> wrote:
> >When attempting to compile test11-pre2, I get the following compile
> >error.
> >
> >arch/i386/mm/mm.o: In function `do_page_fault':
> >arch/i386/mm/mm.o(.text+0x781): undefined reference to `bust_spinlocks'
> >make: *** [vmlinux] Error 1
>
> Oops, wrong patch.
>
> Index: 0-test11-pre2.1/arch/i386/kernel/traps.c
> --- 0-test11-pre2.1/arch/i386/kernel/traps.c Fri, 10 Nov 2000 13:10:37 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
> +++ 0-test11-pre2.1(w)/arch/i386/kernel/traps.c Fri, 10 Nov 2000 16:06:48 +1100 kaos (linux-2.4/A/c/1_traps.c 1.1.2.2.1.1.2.1.2.3.1.2.3.1.1.2 644)
> @@ -382,6 +382,18 @@ static void unknown_nmi_error(unsigned c
> printk("Do you have a strange power saving mode enabled?\n");
> }
>
> +extern spinlock_t console_lock, timerlist_lock;
> +/*
> + * Unlock any spinlocks which will prevent us from getting the
> + * message out (timerlist_lock is acquired through the
> + * console unblank code)
> + */
> +void bust_spinlocks(void)
> +{
> + spin_lock_init(&console_lock);
> + spin_lock_init(&timerlist_lock);
> +}
> +
> #if CONFIG_X86_IO_APIC
>
> int nmi_watchdog = 1;
> @@ -394,19 +406,7 @@ static int __init setup_nmi_watchdog(cha
>
> __setup("nmi_watchdog=", setup_nmi_watchdog);
>
> -extern spinlock_t console_lock, timerlist_lock;
> static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
> -
> -/*
> - * Unlock any spinlocks which will prevent us from getting the
> - * message out (timerlist_lock is aquired through the
> - * console unblank code)
> - */
> -void bust_spinlocks(void)
> -{
> - spin_lock_init(&console_lock);
> - spin_lock_init(&timerlist_lock);
> -}
>
> inline void nmi_watchdog_tick(struct pt_regs * regs)
> {
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, 10 Nov 2000 09:15:54 -0800,
George Anzinger <[email protected]> wrote:
>The notion of releasing a spin lock by initializing it seems IMHO, on
>the face of it, way off.
Normally it would be, but these are NMI and panic messages. The system
is pretty dead at that point, getting the message out is deemed more
important than maintaining the spinlock. BTW, bust_spinlocks is not my
idea, please direct any queries to Andrew Morton. I just fixed a link
error.
George Anzinger wrote:
>
> The notion of releasing a spin lock by initializing it seems IMHO, on
> the face of it, way off. Firstly the protected area is no longer
> protected which could lead to undefined errors/ crashes and secondly,
> any future use of spinlocks to control preemption could have a lot of
> trouble with this, principally because the locker is unknown.
>
> In the case at hand, it would seem that an unlocked path to the console
> is a more correct answer that gives the system a far better chance of
> actually remaining viable.
>
Does bust_spinlocks() muck up the preemptive kernel's spinlock
counting? Would you prefer spin_trylock()/spin_unlock()?
It doesn't matter - if we call bust_spinlocks() the kernel is
known to be dead meat and there is a fsck in your near future.
We are still trying to find out why kumon@fujitsu's 8-way is
crashing on the test10-pre5 sched.c. Looks like it's fixed
in test11-pre2 but we want to know _why_ it's fixed. And at
present each time he hits the bug, his printk() deadlocks.
So bust_spinlocks() is a RAS feature :) A very important one -
it's terrible when your one-in-a-trillion bug happens and there
are no diagnostics.
It's a work-in-progress. There are a lot of things which
can cause printk to deadlock:
- console_lock
- timerlist_lock
- global_irq_lock (console code does global_cli)
- log_wait.lock
- tasklist_lock (printk does wake_up) (*)
- runqueue_lock (printk does wake_up)
I'll be proposing a better patch for this in a few days.
(*) Keith: this explains why you can't do a printk() in
__wake_up_common: printk calls wake_up(). Duh.
Andrew Morton <[email protected]> writes:
> George Anzinger wrote:
> >
> > The notion of releasing a spin lock by initializing it seems IMHO, on
> > the face of it, way off. Firstly the protected area is no longer
> > protected which could lead to undefined errors/ crashes and secondly,
> > any future use of spinlocks to control preemption could have a lot of
> > trouble with this, principally because the locker is unknown.
> >
> > In the case at hand, it would seem that an unlocked path to the console
> > is a more correct answer that gives the system a far better chance of
> > actually remaining viable.
> >
>
> Does bust_spinlocks() muck up the preemptive kernel's spinlock
> counting? Would you prefer spin_trylock()/spin_unlock()?
> It doesn't matter - if we call bust_spinlocks() the kernel is
> known to be dead meat and there is a fsck in your near future.
>
> We are still trying to find out why kumon@fujitsu's 8-way is
> crashing on the test10-pre5 sched.c. Looks like it's fixed
> in test11-pre2 but we want to know _why_ it's fixed. And at
> present each time he hits the bug, his printk() deadlocks.
>
> So bust_spinlocks() is a RAS feature :) A very important one -
> it's terrible when your one-in-a-trillion bug happens and there
> are no diagnostics.
>
> It's a work-in-progress. There are a lot of things which
> can cause printk to deadlock:
>
> - console_lock
> - timerlist_lock
> - global_irq_lock (console code does global_cli)
> - log_wait.lock
> - tasklist_lock (printk does wake_up) (*)
> - runqueue_lock (printk does wake_up)
>
> I'll be proposing a better patch for this in a few days.
Hmm. I would like to suggest we look at non locking variants of
things. i.e. Data structure version changing with swap.
Eric
diff -urP -X patch.exclude linux-2.4.0-test9-kb-rts/kernel/printk.c linux/kernel/printk.c
--- linux-2.4.0-test9-kb-rts/kernel/printk.c Wed Jul 5 11:00:21 2000
+++ linux/kernel/printk.c Thu Nov 2 10:17:20 2000
@@ -312,6 +312,64 @@
return i;
}
+#if defined(CONFIG_KGDB) && defined(CONFIG_PREEMPT)
+asmlinkage int printk_unlocked(const char *fmt, ...)
+{
+ va_list args;
+ int i;
+ char *msg, *p, *buf_end;
+ int line_feed;
+ static signed char msg_level = -1;
+
+ va_start(args, fmt);
+ i = vsprintf(buf + 3, fmt, args); /* hopefully i < sizeof(buf)-4 */
+ buf_end = buf + 3 + i;
+ va_end(args);
+ for (p = buf + 3; p < buf_end; p++) {
+ msg = p;
+ if (msg_level < 0) {
+ if (
+ p[0] != '<' ||
+ p[1] < '0' ||
+ p[1] > '7' ||
+ p[2] != '>'
+ ) {
+ p -= 3;
+ p[0] = '<';
+ p[1] = default_message_loglevel + '0';
+ p[2] = '>';
+ } else
+ msg += 3;
+ msg_level = p[1] - '0';
+ }
+ line_feed = 0;
+ for (; p < buf_end; p++) {
+ log_buf[(log_start+log_size) & LOG_BUF_MASK] = *p;
+ if (log_size < LOG_BUF_LEN)
+ log_size++;
+ else
+ log_start++;
+
+ logged_chars++;
+ if (*p == '\n') {
+ line_feed = 1;
+ break;
+ }
+ }
+ if (msg_level < console_loglevel && console_drivers) {
+ struct console *c = console_drivers;
+ while(c) {
+ if ((c->flags & CON_ENABLED) && c->write)
+ c->write(c, msg, p - msg + line_feed);
+ c = c->next;
+ }
+ }
+ if (line_feed)
+ msg_level = -1;
+ }
+ return i;
+}
+#endif
void console_print(const char *s)
{
struct console *c;