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.

diff --git a/doc/reference.texi b/doc/reference.texi
index 3d7d9bcbd2bbb2df58152293f4b09173cb60aff4..c44988cf810b6e59a5f0d715b20fdd23f3779697 100644 (file)
--- 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.
 
 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.)
 
 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
 
 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 1231168e34224ab3aca2ee50479087ceb0abdf46..c1381d01dabd7107dc878e2763fe29668802cc2e 100644 (file)
--- 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
 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
 
 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.
 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
 @end table
 
 @node Advanced Scheduler FAQ

diff --git a/src/devices/serial.c b/src/devices/serial.c
index f4d0de8114a6818d51730aadedfd116b1570cb40..f64074a0d1482bec6654b34b8e5b5644c8a76fec 100644 (file)
--- 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. */
    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. */
 {
   ASSERT (mode == UNINIT);
   outb (IER_REG, 0);                    /* Turn off all interrupts. */


@@ -83,6 +83,8 @@ serial_init_queue (void)
 {
   enum intr_level old_level;
 
 {
   enum intr_level old_level;
 

+  if (mode == UNINIT)
+    init_poll ();

   ASSERT (mode == POLL);
 
   intr_register_ext (0x20 + 4, serial_interrupt, "serial");
   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 ();
 
 {
   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 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 
       putc_poll (byte); 
     }
   else 

diff --git a/src/devices/serial.h b/src/devices/serial.h
index b187a80240b1ca888d22d165a871cfa6240a7f4c..6e0477850b0b25ae384f033c7a0610c52113af8c 100644 (file)
--- a/src/devices/serial.h
+++ b/src/devices/serial.h
@@ -3,7 +3,6 @@
 
 #include <stdint.h>
 
 
 #include <stdint.h>
 

-void serial_init_poll (void);

 void serial_init_queue (void);
 void serial_putc (uint8_t);
 void serial_flush (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 949dd975eaec8da2ae0e34551b7ca23765a14792..8255747244e641d599d686fadecb167a9fc6b4f7 100644 (file)
--- 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 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
 }
 
 /* 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 ();
 
      that might write to the console. */
   enum intr_level old_level = intr_disable ();
 

+  init ();
+  

   switch (c) 
     {
     case '\n':
   switch (c) 
     {
     case '\n':


@@ -138,3 +147,19 @@ move_cursor (void)
   outw (0x3d4, 0x0f | (cp << 8));
 }
 
   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 190cf2158952123f27a55397b4c8ea6650eab024..59690fb734a1c3abf8e1cc35fff916c6633aa3a6 100644 (file)
--- a/src/devices/vga.h
+++ b/src/devices/vga.h
@@ -1,7 +1,6 @@
 #ifndef DEVICES_VGA_H
 #define DEVICES_VGA_H
 
 #ifndef DEVICES_VGA_H
 #define DEVICES_VGA_H
 

-void vga_init (void);

 void vga_putc (int);
 
 #endif /* devices/vga.h */
 void vga_putc (int);
 
 #endif /* devices/vga.h */

diff --git a/src/lib/kernel/console.c b/src/lib/kernel/console.c
index f62e7f34c246b0e64183d8a40c1621a32b348597..0d031b5b4bdd6a2257f416bdf1fc1ffdbbb4398e 100644 (file)
--- a/src/lib/kernel/console.c
+++ b/src/lib/kernel/console.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include "devices/serial.h"
 #include "devices/vga.h"
 #include <stdio.h>
 #include "devices/serial.h"
 #include "devices/vga.h"

+#include "threads/init.h"

 #include "threads/interrupt.h"
 #include "threads/synch.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;
 
    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().
 /* 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;
 
 /* 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);
 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. */
 }
 
 /* Prints console statistics. */


@@ -63,7 +87,7 @@ console_print_stats (void)
 static void
 acquire_console (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++; 
     {
       if (lock_held_by_current_thread (&console_lock)) 
         console_lock_depth++; 


@@ -76,7 +100,7 @@ acquire_console (void)
 static void
 release_console (void) 
 {
 static void
 release_console (void) 
 {

-  if (!intr_context ()) 
+  if (!intr_context () && use_console_lock) 

     {
       if (console_lock_depth > 0)
         console_lock_depth--;
     {
       if (console_lock_depth > 0)
         console_lock_depth--;


@@ -90,7 +114,9 @@ release_console (void)
 static bool
 console_locked_by_current_thread (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,
 }
 
 /* The standard vprintf() function,

diff --git a/src/lib/kernel/console.h b/src/lib/kernel/console.h
index 788edf2309d1ce94a0c312454d51c3f811ecd634..ab992498bcf4e338948741641ff9a53645ec9c75 100644 (file)
--- 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);
 #define __LIB_KERNEL_CONSOLE_H
 
 void console_init (void);

+void console_panic (void);

 void console_print_stats (void);
 
 #endif /* lib/kernel/console.h */
 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 f6fb9dde1c4bb7090b72c051f8a4e7733f3fb08a..93c395208068801dcdbca3b4dffbec7b64c74712 100644 (file)
--- a/src/lib/kernel/debug.c
+++ b/src/lib/kernel/debug.c
@@ -1,4 +1,5 @@
 #include <debug.h>
 #include <debug.h>

+#include <console.h>

 #include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>


@@ -18,6 +19,7 @@ debug_panic (const char *file, int line, const char *function,
   va_list args;
 
   intr_disable ();
   va_list args;
 
   intr_disable ();

+  console_panic ();

 
   level++;
   if (level == 1) 
 
   level++;
   if (level == 1) 

diff --git a/src/lib/random.c b/src/lib/random.c
index c22414e9b779cd79123fe4390d3d0945b2d02d6c..a4761b6b52c14d3202a783d1e7eb1ab64baba767 100644 (file)
--- a/src/lib/random.c
+++ b/src/lib/random.c
@@ -29,8 +29,7 @@ swap_byte (uint8_t *a, uint8_t *b)
   *b = t;
 }
 
   *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)
 {
 void
 random_init (unsigned seed)
 {


@@ -38,9 +37,6 @@ random_init (unsigned seed)
   int i;
   uint8_t j;
 
   int i;
   uint8_t j;
 

-  if (inited)
-    return;
-  

   for (i = 0; i < 256; i++) 
     s[i] = i;
   for (i = j = 0; i < 256; i++) 
   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;
 
 {
   uint8_t *buf;
 

-  ASSERT (inited);
+  if (!inited)
+    random_init (0);
+

   for (buf = buf_; size-- > 0; buf++)
     {
       uint8_t s_k;
   for (buf = buf_; size-- > 0; buf++)
     {
       uint8_t s_k;

diff --git a/src/threads/init.c b/src/threads/init.c
index 016c24d2de46ca88446b1fcfc6ba74650fc9c94e..5893ce970b21fe12f707e4c3be55767e31c0034f 100644 (file)
--- 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 ();
 
   /* 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);
 
   /* 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 ();
 
   /* Initialize memory system. */
   palloc_init ();

diff --git a/src/threads/thread.c b/src/threads/thread.c
index 1dc34adac7770f6bc53b5e5a1fb6b533ec0043f4..f6768c0225432617c92c40e517a00f75f6fd72b9 100644 (file)
--- 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.
 
 /* 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);
 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().
 
    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
    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.
 }
 
 /* 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) 
 {
 void
 thread_start (void) 
 {

diff --git a/src/threads/thread.h b/src/threads/thread.h
index b45bdb8616f02b66c72a3c455215233de1b4f761..0039560018ddc942fb2f10002ddd4d94e3045b8c 100644 (file)
--- 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.
 
 /* 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);
 extern bool thread_mlfqs;
 
 void thread_init (void);