From 8148b6ee535b85e97f3b5f3a850b70fdfbbcaf2d Mon Sep 17 00:00:00 2001
From: rtm <rtm>
Date: Wed, 12 Jul 2006 11:15:38 +0000
Subject: [PATCH] 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)

---
 Notes      |  4 ++++
 console.c  |  9 +++++++--
 defs.h     |  3 +++
 kalloc.c   |  3 +++
 main.c     | 13 ++++++++++++-
 mp.c       | 11 ++++++++---
 proc.c     | 25 +++++++++++++++++++++++--
 proc.h     |  1 +
 spinlock.c | 32 +++++++++++++++++++++++---------
 spinlock.h |  2 +-
 x86.h      | 14 --------------
 11 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/Notes b/Notes
index b99bfe4..8fab37d 100644
--- 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 3745009..b85a295 100644
--- 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 c5dc4d8..4561ff6 100644
--- 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 969b81d..cc5791d 100644
--- 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 9ba78a9..9015ff7 100644
--- 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 4258aba..068d036 100644
--- 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 4c2586d..53469a1 100644
--- 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 b9ec246..4c5807b 100644
--- 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 9f840a8..bd6bff5 100644
--- 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 ee0a26c..a720b24 100644
--- 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 80a4130..cc809e7 100644
--- 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;