Incorporate new understanding of/with Intel SMP spec.

Dropped cmpxchg in favor of xchg, to match lecture notes.

Use xchg to release lock, for future protection and to
keep gcc from acting clever.

diff --git a/TRICKS b/TRICKS
index b56a38ccc3e697f9e60dde33afa7c787350c9815..688358898928f0c2b68194bf3ba0929c497826db 100644 (file)
--- a/TRICKS
+++ b/TRICKS
@@ -102,5 +102,11 @@ after observing the earlier writes by CPU0.
 So any reads in B are guaranteed to observe the
 effects of writes in A.
 
-Not sure about the second one yet.
+According to the Intel manual behavior spec, the
+second condition requires a serialization instruction
+in release, to avoid reads in A happening after giving
+up lk.  No Intel SMP processor in existence actually
+moves reads down after writes, but the language in
+the spec allows it.  There is no telling whether future
+processors will need it.
 

diff --git a/main.c b/main.c
index 2108d9570867bc3500a0bf3a9ed71c51da5d70e3..6ec6d19bb405924e4af6921c9b61901c2de0c04e 100644 (file)
--- a/main.c
+++ b/main.c
@@ -50,8 +50,9 @@ mpmain(void)
   if(cpu() != mp_bcpu())
     lapic_init(cpu());
   setupsegs(0);
-  cpus[cpu()].booted = 1;
+  xchg(&cpus[cpu()].booted, 1);
 
+  cprintf("cpu%d: scheduling\n");
   scheduler();
 }
 

diff --git a/proc.h b/proc.h
index fa60452834cf99df0dca1ac38797bdf9a7f847cb..502361d7929fea86be42901b76aa648c81dbdb47 100644 (file)
--- a/proc.h
+++ b/proc.h
@@ -56,7 +56,7 @@ struct cpu {
   struct context context;     // Switch here to enter scheduler
   struct taskstate ts;        // Used by x86 to find stack for interrupt
   struct segdesc gdt[NSEGS];  // x86 global descriptor table
-  volatile int booted;        // Has the CPU started?
+  volatile uint booted;        // Has the CPU started?
   int ncli;                   // Depth of pushcli nesting.
   int intena;                 // Were interrupts enabled before pushcli? 
 };

diff --git a/spinlock.c b/spinlock.c
index c00c9787bad852f8e1215f978706e29ad80f2b31..ba5bb4a5e35d62b435ad8cb8d0deeec2c786c1c6 100644 (file)
--- a/spinlock.c
+++ b/spinlock.c
@@ -10,12 +10,6 @@
 
 extern int use_console_lock;
 
-// Barrier to gcc's instruction reordering.
-static void inline gccbarrier(void)
-{
-  asm volatile("" : : : "memory");
-}
-
 void
 initlock(struct spinlock *lock, char *name)
 {
@@ -35,7 +29,10 @@ acquire(struct spinlock *lock)
   if(holding(lock))
     panic("acquire");
 
-  while(cmpxchg(0, 1, &lock->locked) == 1)
+  // The xchg is atomic.
+  // It also serializes, so that reads after acquire are not
+  // reordered before it.  
+  while(xchg(&lock->locked, 1) == 1)
     ;
 
   // Record info about lock acquisition for debugging.
@@ -56,8 +53,12 @@ release(struct spinlock *lock)
   lock->pcs[0] = 0;
   lock->cpu = 0xffffffff;
 
-  gccbarrier();  // Keep gcc from moving lock->locked = 0 earlier.
-  lock->locked = 0;
+  // The xchg serializes, so that reads before release are 
+  // not reordered after it.  (This reordering would be allowed
+  // by the Intel manuals, but does not happen on current 
+  // Intel processors.  The xchg being asm volatile also keeps
+  // gcc from delaying the above assignments.)
+  xchg(&lock->locked, 0);
 
   popcli();
 }

diff --git a/x86.h b/x86.h
index a1c66b5ee42894a948ac6ff4f51e94bbd32091f2..1f2c88131c4b02781c26bca7932e8151759a71f9 100644 (file)
--- a/x86.h
+++ b/x86.h
@@ -96,35 +96,16 @@ write_eflags(uint eflags)
   asm volatile("pushl %0; popfl" : : "r" (eflags));
 }
 
-// XXX: Kill this if not used.
-static inline void
-cpuid(uint info, uint *eaxp, uint *ebxp, uint *ecxp, uint *edxp)
-{
-  uint eax, ebx, ecx, edx;
-
-  asm volatile("cpuid" :
-               "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx) :
-               "a" (info));
-  if(eaxp)
-    *eaxp = eax;
-  if(ebxp)
-    *ebxp = ebx;
-  if(ecxp)
-    *ecxp = ecx;
-  if(edxp)
-    *edxp = edx;
-}
-
 static inline uint
-cmpxchg(uint oldval, uint newval, volatile uint* lock_addr)
+xchg(volatile uint *addr, uint newval)
 {
   uint result;
   
   // The + in "+m" denotes a read-modify-write operand.
-  asm volatile("lock; cmpxchgl %2, %0" :
-                       "+m" (*lock_addr), "=a" (result) :
-                       "r"(newval), "1"(oldval) :
-                       "cc");
+  asm volatile("lock; xchgl %0, %1" :
+               "+m" (*addr), "=a" (result) :
+               "1" (newval) :
+               "cc");
   return result;
 }