From 53a7f5d0952a4595f252247f5ee3d017468eb57e Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 11 Dec 2006 20:40:20 +0000 Subject: [PATCH] Parse command-line options much earlier, so that thread_mlfqs is initialized before thread_init() gets hold of it. Also allow console output much earlier, by doing initialization whenever we call into it first and by only trying to take the console lock after threads have been initialized. Don't try to take the console lock after the kernel panics, to avoid getting nailed so hard by bugs in lock_acquire(). Suggested by Godmar Back. --- doc/reference.texi | 27 +++++++-------------------- doc/threads.texi | 10 +--------- src/devices/serial.c | 10 +++++++--- src/devices/serial.h | 1 - src/devices/vga.c | 35 ++++++++++++++++++++++++++++++----- src/devices/vga.h | 1 - src/lib/kernel/console.c | 34 ++++++++++++++++++++++++++++++---- src/lib/kernel/console.h | 1 + src/lib/kernel/debug.c | 2 ++ src/lib/random.c | 10 ++++------ src/threads/init.c | 20 +++++++------------- src/threads/thread.c | 13 ++----------- src/threads/thread.h | 4 +--- 13 files changed, 92 insertions(+), 76 deletions(-) diff --git a/doc/reference.texi b/doc/reference.texi index 3d7d9bc..c44988c 100644 --- a/doc/reference.texi +++ b/doc/reference.texi @@ -136,30 +136,17 @@ just use @func{memset} to zero it out. The other task of the loader stored it and put it into the @code{ram_pages} variable for later use. -Next, @func{thread_init} initializes the thread system. We will defer -full discussion to our discussion of Pintos threads below. It is -called so early in initialization because the console, initialized -just afterward, tries to use locks, and locks in turn require there to be a -running thread. - -Then we initialize the console so that @func{printf} will work. -@func{main} calls @func{vga_init}, which initializes the VGA text -display and clears the screen. It also calls @func{serial_init_poll} -to initialize the first serial port in ``polling mode,'' that is, -where the kernel busy-waits for the port to be ready for each -character to be output. (We use polling mode until we're ready to enable -interrupts, later.) Finally we initialize the console device and -print a startup message to the console. - -@func{main} calls @func{read_command_line} to break the kernel command +Next, @func{main} calls @func{read_command_line} to break the kernel command line into arguments, then @func{parse_options} to read any options at the beginning of the command line. (Actions specified on the command line execute later.) -@func{main} calls @func{random_init} to initialize the kernel random -number generator. If the user specified @option{-rs} on the -@command{pintos} command line, @func{parse_options} already did -this, but calling it a second time is harmless. +@func{thread_init} initializes the thread system. We will defer full +discussion to our discussion of Pintos threads below. It is called so +early in initialization because a valid thread structure is a +prerequisite for acquiring a lock, and lock acquisition in turn is +important to other Pintos subsystems. Then we initialize the console +and print a startup message to the console. The next block of functions we call initialize the kernel's memory system. @func{palloc_init} sets up the kernel page allocator, which diff --git a/doc/threads.texi b/doc/threads.texi index 1231168..c1381d0 100644 --- a/doc/threads.texi +++ b/doc/threads.texi @@ -524,7 +524,7 @@ scheduler with the @option{-mlfqs} kernel option. Passing this option sets @code{thread_mlfqs}, declared in @file{threads/thread.h}, to true when the options are parsed by @func{parse_options}, which happens -midway through @func{main}. +early in @func{main}. When the 4.4@acronym{BSD} scheduler is enabled, threads no longer directly control their own priorities. The @var{priority} argument to @@ -768,14 +768,6 @@ becomes the higher of the newly set priority or the highest donated priority. When the donations are released, the thread's priority becomes the one set through the function call. This behavior is checked by the @code{priority-donate-lower} test. - -@item Calling @func{printf} in @func{sema_up} or @func{sema_down} reboots! - -@anchor{printf Reboots} -Yes. These functions are called before @func{printf} is ready to go. -You could add a global flag initialized to false and set it to true -just before the first @func{printf} in @func{main}. Then modify -@func{printf} itself to return immediately if the flag isn't set. @end table @node Advanced Scheduler FAQ diff --git a/src/devices/serial.c b/src/devices/serial.c index f4d0de8..f64074a 100644 --- a/src/devices/serial.c +++ b/src/devices/serial.c @@ -63,8 +63,8 @@ static intr_handler_func serial_interrupt; Polling mode busy-waits for the serial port to become free before writing to it. It's slow, but until interrupts have been initialized it's all we can do. */ -void -serial_init_poll (void) +static void +init_poll (void) { ASSERT (mode == UNINIT); outb (IER_REG, 0); /* Turn off all interrupts. */ @@ -83,6 +83,8 @@ serial_init_queue (void) { enum intr_level old_level; + if (mode == UNINIT) + init_poll (); ASSERT (mode == POLL); intr_register_ext (0x20 + 4, serial_interrupt, "serial"); @@ -98,10 +100,12 @@ serial_putc (uint8_t byte) { enum intr_level old_level = intr_disable (); - if (mode == POLL) + if (mode != QUEUE) { /* If we're not set up for interrupt-driven I/O yet, use dumb polling to transmit a byte. */ + if (mode == UNINIT) + init_poll (); putc_poll (byte); } else diff --git a/src/devices/serial.h b/src/devices/serial.h index b187a80..6e04778 100644 --- a/src/devices/serial.h +++ b/src/devices/serial.h @@ -3,7 +3,6 @@ #include -void serial_init_poll (void); void serial_init_queue (void); void serial_putc (uint8_t); void serial_flush (void); diff --git a/src/devices/vga.c b/src/devices/vga.c index 949dd97..8255747 100644 --- a/src/devices/vga.c +++ b/src/devices/vga.c @@ -29,13 +29,20 @@ static void clear_row (size_t y); static void cls (void); static void newline (void); static void move_cursor (void); +static void find_cursor (size_t *x, size_t *y); -/* Initializes the VGA text display and clears the screen. */ -void -vga_init (void) +/* Initializes the VGA text display. */ +static void +init (void) { - fb = ptov (0xb8000); - cls (); + /* Already initialized? */ + static bool inited; + if (!inited) + { + fb = ptov (0xb8000); + find_cursor (&cx, &cy); + inited = true; + } } /* Writes C to the VGA text display, interpreting control @@ -47,6 +54,8 @@ vga_putc (int c) that might write to the console. */ enum intr_level old_level = intr_disable (); + init (); + switch (c) { case '\n': @@ -138,3 +147,19 @@ move_cursor (void) outw (0x3d4, 0x0f | (cp << 8)); } +/* Reads the current hardware cursor position into (*X,*Y). */ +static void +find_cursor (size_t *x, size_t *y) +{ + /* See [FREEVGA] under "Manipulating the Text-mode Cursor". */ + uint16_t cp; + + outb (0x3d4, 0x0e); + cp = inb (0x3d5) << 8; + + outb (0x3d4, 0x0f); + cp |= inb (0x3d5); + + *x = cp % COL_CNT; + *y = cp / COL_CNT; +} diff --git a/src/devices/vga.h b/src/devices/vga.h index 190cf21..59690fb 100644 --- a/src/devices/vga.h +++ b/src/devices/vga.h @@ -1,7 +1,6 @@ #ifndef DEVICES_VGA_H #define DEVICES_VGA_H -void vga_init (void); void vga_putc (int); #endif /* devices/vga.h */ diff --git a/src/lib/kernel/console.c b/src/lib/kernel/console.c index f62e7f3..0d031b5 100644 --- a/src/lib/kernel/console.c +++ b/src/lib/kernel/console.c @@ -3,6 +3,7 @@ #include #include "devices/serial.h" #include "devices/vga.h" +#include "threads/init.h" #include "threads/interrupt.h" #include "threads/synch.h" @@ -16,6 +17,19 @@ static void putchar_have_lock (uint8_t c); from mixing their output, which looks confusing. */ static struct lock console_lock; +/* True in ordinary circumstances: we want to use the console + lock to avoid mixing output between threads, as explained + above. + + False in early boot before the point that locks are functional + or the console lock has been initialized, or after a kernel + panics. In the former case, taking the lock would cause an + assertion failure, which in turn would cause a panic, turning + it into the latter case. In the latter case, if it is a buggy + lock_acquire() implementation that caused the panic, we'll + likely just recurse. */ +static bool use_console_lock; + /* It's possible, if you add enough debug output to Pintos, to try to recursively grab console_lock from a single thread. As a real example, I added a printf() call to palloc_free(). @@ -45,11 +59,21 @@ static int console_lock_depth; /* Number of characters written to console. */ static int64_t write_cnt; -/* Initializes the console. */ +/* Enable console locking. */ void console_init (void) { lock_init (&console_lock); + use_console_lock = true; +} + +/* Notifies the console that a kernel panic is underway, + which warns it to avoid trying to take the console lock from + now on. */ +void +console_panic (void) +{ + use_console_lock = false; } /* Prints console statistics. */ @@ -63,7 +87,7 @@ console_print_stats (void) static void acquire_console (void) { - if (!intr_context ()) + if (!intr_context () && use_console_lock) { if (lock_held_by_current_thread (&console_lock)) console_lock_depth++; @@ -76,7 +100,7 @@ acquire_console (void) static void release_console (void) { - if (!intr_context ()) + if (!intr_context () && use_console_lock) { if (console_lock_depth > 0) console_lock_depth--; @@ -90,7 +114,9 @@ release_console (void) static bool console_locked_by_current_thread (void) { - return intr_context () || lock_held_by_current_thread (&console_lock); + return (intr_context () + || !use_console_lock + || lock_held_by_current_thread (&console_lock)); } /* The standard vprintf() function, diff --git a/src/lib/kernel/console.h b/src/lib/kernel/console.h index 788edf2..ab99249 100644 --- a/src/lib/kernel/console.h +++ b/src/lib/kernel/console.h @@ -2,6 +2,7 @@ #define __LIB_KERNEL_CONSOLE_H void console_init (void); +void console_panic (void); void console_print_stats (void); #endif /* lib/kernel/console.h */ diff --git a/src/lib/kernel/debug.c b/src/lib/kernel/debug.c index f6fb9dd..93c3952 100644 --- a/src/lib/kernel/debug.c +++ b/src/lib/kernel/debug.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -18,6 +19,7 @@ debug_panic (const char *file, int line, const char *function, va_list args; intr_disable (); + console_panic (); level++; if (level == 1) diff --git a/src/lib/random.c b/src/lib/random.c index c22414e..a4761b6 100644 --- a/src/lib/random.c +++ b/src/lib/random.c @@ -29,8 +29,7 @@ swap_byte (uint8_t *a, uint8_t *b) *b = t; } -/* Initializes the PRNG with the given SEED. - Does nothing if the PRNG has already been initialized. */ +/* Initializes or reinitializes the PRNG with the given SEED. */ void random_init (unsigned seed) { @@ -38,9 +37,6 @@ random_init (unsigned seed) int i; uint8_t j; - if (inited) - return; - for (i = 0; i < 256; i++) s[i] = i; for (i = j = 0; i < 256; i++) @@ -59,7 +55,9 @@ random_bytes (void *buf_, size_t size) { uint8_t *buf; - ASSERT (inited); + if (!inited) + random_init (0); + for (buf = buf_; size-- > 0; buf++) { uint8_t s_k; diff --git a/src/threads/init.c b/src/threads/init.c index 016c24d..5893ce9 100644 --- a/src/threads/init.c +++ b/src/threads/init.c @@ -71,23 +71,17 @@ main (void) /* Clear BSS and get machine's RAM size. */ ram_init (); - /* Initialize ourselves as a thread so we can use locks. */ - thread_init (); - - /* Initialize the console so we can use printf(). */ - vga_init (); - serial_init_poll (); - console_init (); - - /* Greet user. */ - printf ("Pintos booting with %'zu kB RAM...\n", ram_pages * PGSIZE / 1024); - /* Break command line into arguments and parse options. */ argv = read_command_line (); argv = parse_options (argv); - /* Set random seed if parse_options() didn't. */ - random_init (0); + /* Initialize ourselves as a thread so we can use locks, + then enable console locking. */ + thread_init (); + console_init (); + + /* Greet user. */ + printf ("Pintos booting with %'zu kB RAM...\n", ram_pages * PGSIZE / 1024); /* Initialize memory system. */ palloc_init (); diff --git a/src/threads/thread.c b/src/threads/thread.c index 1dc34ad..f6768c0 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -52,9 +52,7 @@ static unsigned thread_ticks; /* # of timer ticks since last yield. */ /* If false (default), use round-robin scheduler. If true, use multi-level feedback queue scheduler. - Controlled by kernel command-line options "-o mlfqs". - Note that the command line is not parsed until well after - thread_init() is called. */ + Controlled by kernel command-line option "-o mlfqs". */ bool thread_mlfqs; static void kernel_thread (thread_func *, void *aux); @@ -80,10 +78,6 @@ static tid_t allocate_tid (void); allocator before trying to create any threads with thread_create(). - The kernel command line is not parsed until *after* this - function returns, so that when this function runs, - thread_mlfqs is always false. - It is not safe to call thread_current() until this function finishes. */ void @@ -102,10 +96,7 @@ thread_init (void) } /* Starts preemptive thread scheduling by enabling interrupts. - Also creates the idle thread. - - By the time this function runs, thread_mlfqs has been properly - initialized to its final value. */ + Also creates the idle thread. */ void thread_start (void) { diff --git a/src/threads/thread.h b/src/threads/thread.h index b45bdb8..0039560 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -103,9 +103,7 @@ struct thread /* If false (default), use round-robin scheduler. If true, use multi-level feedback queue scheduler. - Controlled by kernel command-line options "-o mlfqs". - Note that the command line is not parsed until well after - thread_init() is called. */ + Controlled by kernel command-line option "-o mlfqs". */ extern bool thread_mlfqs; void thread_init (void); -- 2.30.2