i think my cmpxchg use was wrong in acquire

nesting cli/sti: release shouldn't always enable interrupts
separate setup of lapic from starting of other cpus, so cpu() works earlier
flag to disable locking in console output
make locks work even when curproc==0
(still crashes in clock interrupt)

diff --git a/Notes b/Notes
index b99bfe4ee54673adb9af6808f0cddb5de3bd2dc5..8fab37d3229d925ccd45345d9856ed875225d2a6 100644 (file)
--- a/Notes
+++ b/Notes
@@ -125,6 +125,10 @@ in general, the table locks protect both free-ness and
 
 why can't i get a lock in console code?
   always triple fault
+  because release turns on interrupts!
+  a bad idea very early in main()
+  but mp_init() calls cprintf
+
 lock code shouldn't call cprintf...
 ide_init doesn't work now?
 and IOAPIC: read from unsupported address

diff --git a/console.c b/console.c
index 37450093a00f941cb01a8f4297ec6b58cb8efe03..b85a295faa30d9a4baa3f796bd4608c1a8ce0700 100644 (file)
--- a/console.c
+++ b/console.c
@@ -4,6 +4,7 @@
 #include "spinlock.h"
 
 struct spinlock console_lock;
+int use_printf_lock = 0;
 
 /*
  * copy console output to parallel port, which you can tell
@@ -29,7 +30,8 @@ cons_putc(int c)
   unsigned short *crt = (unsigned short *) 0xB8000; // base of CGA memory
   int ind;
 
-  //acquire(&console_lock);
+  if(use_printf_lock)
+    acquire(&console_lock);
 
   lpt_putc(c);
 
@@ -62,7 +64,8 @@ cons_putc(int c)
   outb(crtport, 15);
   outb(crtport + 1, ind);
 
-  //release(&console_lock);
+  if(use_printf_lock)
+    release(&console_lock);
 }
 
 void
@@ -127,6 +130,8 @@ cprintf(char *fmt, ...)
 void
 panic(char *s)
 {
+  use_printf_lock = 0;
+  cprintf("panic: ");
   cprintf(s, 0);
   cprintf("\n", 0);
   while(1)

diff --git a/defs.h b/defs.h
index c5dc4d8bcbc817de75695956ed254fd27ac6ea6e..4561ff63bf0bc12acfe72baaea2f648ed7b7d822 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -19,6 +19,8 @@ void wakeup(void *);
 void scheduler(void);
 void proc_exit(void);
 void yield(void);
+void cli(void);
+void sti(void);
 
 // swtch.S
 struct jmpbuf;
@@ -46,6 +48,7 @@ void pic_init(void);
 
 // mp.c
 void mp_init(void);
+void mp_startthem(void);
 int cpu(void);
 int mp_isbcpu(void);
 void lapic_init(int);

diff --git a/kalloc.c b/kalloc.c
index 969b81d0218c29b8b8c4ec39b73421a2a9c902f2..cc5791d87e59b1a397ca7b5771f1c7e667c82f84 100644 (file)
--- a/kalloc.c
+++ b/kalloc.c
@@ -10,6 +10,9 @@
 #include "param.h"
 #include "types.h"
 #include "defs.h"
+#include "param.h"
+#include "mmu.h"
+#include "proc.h"
 #include "spinlock.h"
 
 struct spinlock kalloc_lock;

diff --git a/main.c b/main.c
index 9ba78a9877b37423f5212b1c6560094929cebad7..9015ff7276bb9a8d45f43cd1d8b2de13dea40e98 100644 (file)
--- a/main.c
+++ b/main.c
@@ -8,6 +8,7 @@
 #include "syscall.h"
 #include "elf.h"
 #include "param.h"
+#include "spinlock.h"
 
 extern char edata[], end[];
 extern int acpu;
@@ -15,23 +16,33 @@ extern char _binary_user1_start[], _binary_user1_size[];
 extern char _binary_usertests_start[], _binary_usertests_size[];
 extern char _binary_userfs_start[], _binary_userfs_size[];
 
+extern use_printf_lock;
+
 int
 main()
 {
   struct proc *p;
 
   if (acpu) {
+    cpus[cpu()].clis = 1;
     cprintf("an application processor\n");
     idtinit(); // CPU's idt
     lapic_init(cpu());
     lapic_timerinit();
     lapic_enableintr();
+    sti();
     scheduler();
   }
   acpu = 1;
+
   // clear BSS
   memset(edata, 0, end - edata);
 
+  mp_init(); // just set up apic so cpu() works
+  use_printf_lock = 1;
+
+  cpus[cpu()].clis = 1; // cpu starts as if we had called cli()
+
   cprintf("\nxV6\n\n");
 
   pic_init(); // initialize PIC
@@ -56,7 +67,7 @@ main()
   p->ppid = 0;
   setupsegs(p);
 
-  mp_init(); // multiprocessor
+  mp_startthem();
 
   // turn on timer and enable interrupts on the local APIC
   lapic_timerinit();

diff --git a/mp.c b/mp.c
index 4258aba08b7d41f1dd8dfe7ce6358bbb5cf1a6eb..068d0362eaa8309a58411a665ae73f7c147ff610 100644 (file)
--- a/mp.c
+++ b/mp.c
@@ -325,8 +325,6 @@ mp_init()
   struct MPCTB *mpctb;
   struct MPPE *proc;
   struct MPBE *bus;
-  int c;
-  extern int main();
   int i;
 
   ncpu = 0;
@@ -386,13 +384,20 @@ mp_init()
   
   lapic_init(bcpu-cpus);
   cprintf("ncpu: %d boot %d\n", ncpu, bcpu-cpus);
+}
 
+void
+mp_startthem()
+{
   extern uint8_t _binary_bootother_start[], _binary_bootother_size[];
+  extern int main();
+  int c;
+
   memmove((void *) APBOOTCODE,_binary_bootother_start, 
          (uint32_t) _binary_bootother_size);
 
   for(c = 0; c < ncpu; c++){
-    if (cpus+c == bcpu) continue;
+    if (c == cpu()) continue;
     cprintf ("starting processor %d\n", c);
     *(unsigned *)(APBOOTCODE-4) = (unsigned) (cpus[c].mpstack) + MPSTACK; // tell it what to use for %esp
     *(unsigned *)(APBOOTCODE-8) = (unsigned)&main; // tell it where to jump to

diff --git a/proc.c b/proc.c
index 4c2586d3b5a307f0425d9d9b80312d62990048d6..53469a1baf2d8f4579c0cbdbf7e616fcffef5028 100644 (file)
--- a/proc.c
+++ b/proc.c
@@ -148,6 +148,7 @@ scheduler(void)
 
     if(i < NPROC){
       np->state = RUNNING;
+      release(&proc_table_lock);
       break;
     }
     
@@ -157,8 +158,6 @@ scheduler(void)
 
   cpus[cpu()].lastproc = np;
   curproc[cpu()] = np;
-  
-  release(&proc_table_lock);
 
   // h/w sets busy bit in TSS descriptor sometimes, and faults
   // if it's set in LTR. so clear tss descriptor busy bit.
@@ -252,3 +251,25 @@ proc_exit()
   // switch into scheduler
   swtch(ZOMBIE);
 }
+
+// disable interrupts
+void
+cli(void)
+{
+  cpus[cpu()].clis += 1;
+  if(cpus[cpu()].clis == 1)
+    __asm __volatile("cli");
+}
+
+// enable interrupts
+void
+sti(void)
+{
+  if(cpus[cpu()].clis < 1){
+    cprintf("cpu %d clis %d\n", cpu(), cpus[cpu()].clis);
+    panic("sti");
+  }
+  cpus[cpu()].clis -= 1;
+  if(cpus[cpu()].clis < 1)
+    __asm __volatile("sti");
+}

diff --git a/proc.h b/proc.h
index b9ec246f8c4eb4be8c0db9e0e801b2d48c6a5080..4c5807bbdf964840b885276af4f4b93fc609225f 100644 (file)
--- a/proc.h
+++ b/proc.h
@@ -69,6 +69,7 @@ struct cpu {
   struct jmpbuf jmpbuf;
   char mpstack[MPSTACK]; // per-cpu start-up stack, only used to get into main()
   struct proc *lastproc;  // last proc scheduled on this cpu (never NULL)
+  int clis; // cli() nesting depth
 };
 
 extern struct cpu cpus[NCPU];

diff --git a/spinlock.c b/spinlock.c
index 9f840a88752687d9d2509000ac8577cf3cfca77a..bd6bff57159615e8a484db4f34d0e1cd324bcea4 100644 (file)
--- a/spinlock.c
+++ b/spinlock.c
@@ -8,6 +8,8 @@
 
 #define DEBUG 0
 
+extern use_printf_lock;
+
 int getcallerpc(void *v) {
   return ((int*)v)[-1];
 }
@@ -15,37 +17,49 @@ int getcallerpc(void *v) {
 void
 acquire(struct spinlock * lock)
 {
-  struct proc * cp = curproc[cpu()];
+  unsigned who;
+
+  if(curproc[cpu()])
+    who = (unsigned) curproc[cpu()];
+  else
+    who = cpu() + 1;
 
-  // on a real machine there would be a memory barrier here
   if(DEBUG) cprintf("cpu%d: acquiring at %x\n", cpu(), getcallerpc(&lock));
-  if (cp && lock->p == cp && lock->locked){
+
+  if (lock->who == who && lock->locked){
     lock->count += 1;
   } else { 
     cli();
-    while ( cmpxchg(0, 1, &lock->locked) != 1 ) { ; }
+    // if we get the lock, eax will be zero
+    // if we don't get the lock, eax will be one
+    while ( cmpxchg(0, 1, &lock->locked) == 1 ) { ; }
     lock->locker_pc = getcallerpc(&lock);
     lock->count = 1;
-    lock->p = cp;
+    lock->who = who;
   }
+
   if(DEBUG) cprintf("cpu%d: acquired at %x\n", cpu(), getcallerpc(&lock));
 }
 
 void
 release(struct spinlock * lock)
 {
-  struct proc * cp = curproc[cpu()];
+  unsigned who;
+
+  if(curproc[cpu()])
+    who = (unsigned) curproc[cpu()];
+  else
+    who = cpu() + 1;
 
   if(DEBUG) cprintf ("cpu%d: releasing at %x\n", cpu(), getcallerpc(&lock));
 
-  if(lock->p != cp || lock->count < 1 || lock->locked != 1)
+  if(lock->who != who || lock->count < 1 || lock->locked != 1)
     panic("release");
 
   lock->count -= 1;
   if(lock->count < 1){
-    lock->p = 0;
+    lock->who = 0;
     cmpxchg(1, 0, &lock->locked);
     sti();
-    // on a real machine there would be a memory barrier here
   }
 }

diff --git a/spinlock.h b/spinlock.h
index ee0a26c91c5b4848f1fc3b0950eb22213afc247d..a720b2462019b77ecbd42a5ee8f01bd08da0e6b8 100644 (file)
--- a/spinlock.h
+++ b/spinlock.h
@@ -1,6 +1,6 @@
 struct spinlock {
   unsigned int locked;
-  struct proc *p;
+  unsigned who;
   int count;
   unsigned locker_pc;
 };

diff --git a/x86.h b/x86.h
index 80a4130289e531624d6613256c794998dedb6fe9..cc809e7c032b13e0955e255574fbf8177d5714aa 100644 (file)
--- a/x86.h
+++ b/x86.h
@@ -304,20 +304,6 @@ read_tsc(void)
         return tsc;
 }
 
-// disable interrupts
-static __inline void
-cli(void)
-{
-        __asm __volatile("cli");
-}
-
-// enable interrupts
-static __inline void
-sti(void)
-{
-        __asm __volatile("sti");
-}
-
 struct PushRegs {
     /* registers as pushed by pusha */
     uint32_t reg_edi;