From eb718e3b5a5470b11e58dcc652f79e115272257a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 11 Sep 2004 22:43:06 +0000 Subject: [PATCH] Really make it safe to call printf() from any context. --- src/devices/intq.c | 71 ++++++++++++++------------------------------ src/devices/intq.h | 26 ++++------------ src/devices/kbd.c | 7 +++-- src/devices/serial.c | 49 +++++++++++++++++++++++------- src/devices/serial.h | 1 + 5 files changed, 71 insertions(+), 83 deletions(-) diff --git a/src/devices/intq.c b/src/devices/intq.c index 7f73991..8754c85 100644 --- a/src/devices/intq.c +++ b/src/devices/intq.c @@ -3,7 +3,6 @@ #include "threads/thread.h" static int next (int pos); -static bool owned_by_current_thread (const struct intq *); static void wait (struct intq *q, struct thread **waiter); static void signal (struct intq *q, struct thread **waiter); @@ -17,29 +16,6 @@ intq_init (struct intq *q, const char *name) q->head = q->tail = 0; } -/* Locks out other threads from Q (with Q's lock) and interrupt - handlers (by disabling interrupts). */ -void -intq_lock (struct intq *q) -{ - ASSERT (!intr_context ()); - ASSERT (!owned_by_current_thread (q)); - - lock_acquire (&q->lock); - q->old_level = intr_disable (); -} - -/* Unlocks Q. */ -void -intq_unlock (struct intq *q) -{ - ASSERT (!intr_context ()); - ASSERT (owned_by_current_thread (q)); - - lock_release (&q->lock); - intr_set_level (q->old_level); -} - /* Returns true if Q is empty, false otherwise. */ bool intq_empty (const struct intq *q) @@ -58,18 +34,22 @@ intq_full (const struct intq *q) /* Removes a byte from Q and returns it. Q must not be empty if called from an interrupt handler. - Otherwise, if Q is empty, first waits until a byte is added. - Either Q must be locked or we must be in an interrupt - handler. */ + Otherwise, if Q is empty, first sleeps until a byte is + added. */ uint8_t intq_getc (struct intq *q) { uint8_t byte; - ASSERT (owned_by_current_thread (q)); - while (intq_empty (q)) - wait (q, &q->not_empty); - + ASSERT (intr_get_level () == INTR_OFF); + while (intq_empty (q)) + { + ASSERT (!intr_context ()); + lock_acquire (&q->lock); + wait (q, &q->not_empty); + lock_release (&q->lock); + } + byte = q->buf[q->tail]; q->tail = next (q->tail); signal (q, &q->not_full); @@ -78,15 +58,19 @@ intq_getc (struct intq *q) /* Adds BYTE to the end of Q. Q must not be full if called from an interrupt handler. - Otherwise, if Q is full, first waits until a byte is removed. - Either Q must be locked or we must be in an interrupt - handler. */ + Otherwise, if Q is full, first sleeps until a byte is + removed. */ void intq_putc (struct intq *q, uint8_t byte) { - ASSERT (owned_by_current_thread (q)); + ASSERT (intr_get_level () == INTR_OFF); while (intq_full (q)) - wait (q, &q->not_full); + { + ASSERT (!intr_context ()); + lock_acquire (&q->lock); + wait (q, &q->not_full); + lock_release (&q->lock); + } q->buf[q->head] = byte; q->head = next (q->head); @@ -100,24 +84,13 @@ next (int pos) return (pos + 1) % INTQ_BUFSIZE; } -/* Returns true if Q is "owned by" the current thread; that is, - if Q is locked by the current thread or if we're in an - external interrupt handler. */ -static bool -owned_by_current_thread (const struct intq *q) -{ - return (intr_context () - || (lock_held_by_current_thread (&q->lock) - && intr_get_level () == INTR_OFF)); -} - /* WAITER must be the address of Q's not_empty or not_full member. Waits until the given condition is true. */ static void wait (struct intq *q, struct thread **waiter) { ASSERT (!intr_context ()); - ASSERT (owned_by_current_thread (q)); + ASSERT (intr_get_level () == INTR_OFF); ASSERT ((waiter == &q->not_empty && intq_empty (q)) || (waiter == &q->not_full && intq_full (q))); @@ -132,7 +105,7 @@ wait (struct intq *q, struct thread **waiter) static void signal (struct intq *q, struct thread **waiter) { - ASSERT (owned_by_current_thread (q)); + ASSERT (intr_get_level () == INTR_OFF); ASSERT ((waiter == &q->not_empty && !intq_empty (q)) || (waiter == &q->not_full && !intq_full (q))); diff --git a/src/devices/intq.h b/src/devices/intq.h index 161d7eb..2b3edea 100644 --- a/src/devices/intq.h +++ b/src/devices/intq.h @@ -7,23 +7,14 @@ /* An "interrupt queue", a circular buffer shared between kernel threads and external interrupt handlers. - A kernel thread that touches an interrupt queue must bracket - its accesses with calls to intq_lock() and intq_unlock(). - These functions take a lock associated with the queue (which - locks out other kernel threads) and disable interrupts (which - locks out interrupt handlers). - - An external interrupt handler that touches an interrupt queue - need not take any special precautions. Interrupts are - disabled in an external interrupt handler, so other code will - not interfere. The interrupt cannot occur during an update to - the interrupt queue by a kernel thread because kernel threads - disable interrupts while touching interrupt queues. + These functions can be called from kernel threads or from + external interrupt handlers. Except for intq_init(), + interrupts must be off in either case. Incidentally, this has the structure of a "monitor". Normally we'd use locks and condition variables from threads/synch.h to - implement a monitor. Unfortunately, those are intended only - to protect kernel threads from one another, not from interrupt + implement a monitor, but those are intended only to protect + kernel threads from one another, not from interrupt handlers. */ /* Queue buffer size, in bytes. */ @@ -32,11 +23,8 @@ /* A circular queue of bytes. */ struct intq { - /* Mutual exclusion. */ - enum intr_level old_level; /* Excludes interrupt handlers. */ - struct lock lock; /* Excludes kernel threads. */ - /* Waiting threads. */ + struct lock lock; /* Only one thread may wait at once. */ struct thread *not_full; /* Thread waiting for not-full condition. */ struct thread *not_empty; /* Thread waiting for not-empty condition. */ @@ -47,8 +35,6 @@ struct intq }; void intq_init (struct intq *, const char *); -void intq_lock (struct intq *); -void intq_unlock (struct intq *); bool intq_empty (const struct intq *); bool intq_full (const struct intq *); uint8_t intq_getc (struct intq *); diff --git a/src/devices/kbd.c b/src/devices/kbd.c index ea98358..9cd2840 100644 --- a/src/devices/kbd.c +++ b/src/devices/kbd.c @@ -40,11 +40,12 @@ kbd_init (void) uint8_t kbd_getc (void) { + enum intr_level old_level; uint8_t key; - - intq_lock (&buffer); + + old_level = intr_disable (); key = intq_getc (&buffer); - intq_unlock (&buffer); + intr_set_level (old_level); return key; } diff --git a/src/devices/serial.c b/src/devices/serial.c index 64ce90f..230a320 100644 --- a/src/devices/serial.c +++ b/src/devices/serial.c @@ -15,6 +15,7 @@ static enum { UNINIT, POLL, QUEUE } mode; static struct intq txq; static void set_serial (int bps, int bits, enum parity_type parity, int stop); +static void putc_poll (uint8_t); static void write_ier (void); static intr_handler_func serial_interrupt; @@ -30,6 +31,7 @@ serial_init_poll (void) outb (FCR_REG, 0); /* Disable FIFO. */ set_serial (9600, 8, NONE, 1); /* 9600 bps, N-8-1. */ outb (MCR_REG, 8); /* Turn on OUT2 output line. */ + intq_init (&txq, "serial xmit"); mode = POLL; } @@ -40,7 +42,6 @@ void serial_init_queue (void) { ASSERT (mode == POLL); - intq_init (&txq, "serial xmit"); intr_register (0x20 + 4, 0, INTR_OFF, serial_interrupt, "serial"); mode = QUEUE; } @@ -49,23 +50,36 @@ serial_init_queue (void) void serial_putc (uint8_t byte) { - if (mode == POLL || intr_context ()) + enum intr_level old_level = intr_disable (); + + if (mode == POLL || old_level == INTR_OFF) { - /* Poll the serial port until it's ready for a byte, and - then transmit. */ - while ((inb (LSR_REG) & LSR_THRE) == 0) - continue; - outb (THR_REG, byte); + /* If we're not set up for interrupt-driven I/O yet, + or if interrupts are off, + use dumb polling for serial I/O. */ + serial_flush (); + putc_poll (byte); } else { - /* Lock the queue, add a byte, and update the interrupt - enable register. */ - intq_lock (&txq); + /* Otherwise, queue a byte and update the interrupt enable + register. */ intq_putc (&txq, byte); write_ier (); - intq_unlock (&txq); } + + intr_set_level (old_level); +} + +/* Flushes anything in the serial buffer out the port in polling + mode. */ +void +serial_flush (void) +{ + enum intr_level old_level = intr_disable (); + while (!intq_empty (&txq)) + putc_poll (intq_getc (&txq)); + intr_set_level (old_level); } /* Configures the serial port for BPS bits per second, BITS bits @@ -95,6 +109,19 @@ write_ier (void) outb (IER_REG, intq_empty (&txq) ? 0 : IER_XMIT); } + +/* Polls the serial port until it's ready, + and then transmits BYTE. */ +static void +putc_poll (uint8_t byte) +{ + ASSERT (intr_get_level () == INTR_OFF); + + while ((inb (LSR_REG) & LSR_THRE) == 0) + continue; + outb (THR_REG, byte); +} + /* Serial interrupt handler. As long as we have a byte to transmit, and the hardware is ready to accept a byte for transmission, diff --git a/src/devices/serial.h b/src/devices/serial.h index 3500ffa..6d49c11 100644 --- a/src/devices/serial.h +++ b/src/devices/serial.h @@ -6,5 +6,6 @@ void serial_init_poll (void); void serial_init_queue (void); void serial_putc (uint8_t); +void serial_flush (void); #endif /* devices/serial.h */ -- 2.30.2