Documenting and slight restructure in spinlock.c

Makefile

@@ -32,7 +32,7 @@ OBJS = \
 
 # riscv64-unknown-elf- or riscv64-linux-gnu-
 # perhaps in /opt/riscv/bin
-#TOOLPREFIX = 
+#TOOLPREFIX =
 
 # Try to infer the correct TOOLPREFIX if not set
 ifndef TOOLPREFIX
@@ -73,6 +73,8 @@ endif
 
 LDFLAGS = -z max-page-size=4096
 
+all: kernel world fs
+
 $K/kernel: $(OBJS) $K/kernel.ld $U/initcode
 	$(LD) $(LDFLAGS) -T $K/kernel.ld -o $K/kernel $(OBJS) 
 	$(OBJDUMP) -S $K/kernel > $K/kernel.asm
@@ -139,7 +141,7 @@ fs.img: mkfs/mkfs README.md $(UPROGS)
 
 -include kernel/*.d user/*.d
 
-clean: 
+clean:
 	rm -f *.tex *.dvi *.idx *.aux *.log *.ind *.ilg \
 	*/*.o */*.d */*.asm */*.sym \
 	$U/initcode $U/initcode.out $K/kernel fs.img \

kernel/file.c

@@ -3,12 +3,10 @@
 //
 
 #include "types.h"
-#include "riscv.h"
 #include "defs.h"
 #include "param.h"
 #include "fs.h"
 #include "spinlock.h"
-#include "sleeplock.h"
 #include "file.h"
 #include "stat.h"
 #include "proc.h"

kernel/main.c

@@ -1,7 +1,3 @@
-#include "types.h"
-#include "param.h"
-#include "memlayout.h"
-#include "riscv.h"
 #include "defs.h"
 
 volatile static int started = 0;

kernel/proc.h

@@ -5,7 +5,7 @@
 #include "riscv.h"
 #include "spinlock.h"
 
-// Saved registers for kernel context switches.
+/** Saved registers for kernel context switches. */
 struct context {
   u64 ra;
   u64 sp;
@@ -25,7 +25,7 @@ struct context {
   u64 s11;
 };
 
-// Per-CPU state.
+/** Per-CPU state. */
 struct cpu {
   struct proc   *proc;    // The process running on this cpu, or null.
   struct context context; // swtch() here to enter scheduler().
@@ -88,7 +88,7 @@ struct trapframe {
 
 enum procstate { UNUSED, USED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
 
-// Per-process state
+/** Per-process state */
 struct proc {
   struct spinlock lock;
 

kernel/sleeplock.c

@@ -1,11 +1,6 @@
 // Sleeping locks
 
-#include "types.h"
-#include "riscv.h"
 #include "defs.h"
-#include "param.h"
-#include "memlayout.h"
-#include "spinlock.h"
 #include "proc.h"
 #include "sleeplock.h"
 

kernel/spinlock.c

@@ -1,13 +1,44 @@
-// Mutual exclusion spin locks.
+/**
+ * Mutual exclusion spin locks.
+ * (Not mutexes as these are spinning locks).
+ */
 
-#include "types.h"
-#include "param.h"
-#include "memlayout.h"
 #include "spinlock.h"
 #include "riscv.h"
 #include "proc.h"
 #include "defs.h"
 
+/**
+ * The aquire() and release() functions control ownership of the lock.
+ * To perform these operations, modern CPU's provide atomic instructions
+ * that prevent the cores from stepping on each other's toes, otherwise known
+ * as a deadlock.
+ *
+ * GCC provides a set of built-in functions that allow you to use atomic
+ * instructions in an architecture-independent way. These functions are
+ * defined in the GCC manual:
+ *
+ *   See: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html
+ *   See: https://en.wikipedia.org/wiki/Memory_barrier
+ *
+ * On RISC-V, sync_lock_test_and_set turns into an atomic swap:
+ *   a5 = 1
+ *   s1 = &lk->locked
+ *   amoswap.w.aq a5, a5, (s1)
+ *
+ * On RISC-V, sync_lock_release turns into an atomic swap:
+ *   s1 = &lk->locked
+ *   amoswap.w zero, zero, (s1)
+ *
+ *   __sync_synchronize();
+ *
+ * This function tells the C compiler and the processor to not move loads or stores
+ * past this point, to ensure that the critical section's memory
+ * references happen strictly after the lock is acquired/locked.
+ * On RISC-V, this emits a fence instruction.
+ */
+
+/** Initialize spinlock */
 void
 initlock(struct spinlock *lk, char *name)
 {
@@ -16,57 +47,41 @@ initlock(struct spinlock *lk, char *name)
   lk->cpu = 0;
 }
 
-// Acquire the lock.
-// Loops (spins) until the lock is acquired.
+/**
+ * Acquire the lock.
+ * Loops (spins) until the lock is acquired.
+ * Panics if the lock is already held by this cpu.
+ */
 void
 acquire(struct spinlock *lk)
 {
   push_off(); // disable interrupts to avoid deadlock.
-  if(holding(lk))
+
+  if(holding(lk)) // If the lock is already held, panic.
     panic("acquire");
 
-  // On RISC-V, sync_lock_test_and_set turns into an atomic swap:
-  //   a5 = 1
-  //   s1 = &lk->locked
-  //   amoswap.w.aq a5, a5, (s1)
+  // See file header for details
   while(__sync_lock_test_and_set(&lk->locked, 1) != 0)
     ;
-
-  // Tell the C compiler and the processor to not move loads or stores
-  // past this point, to ensure that the critical section's memory
-  // references happen strictly after the lock is acquired.
-  // On RISC-V, this emits a fence instruction.
-  __sync_synchronize();
+  __sync_synchronize(); // No loads/stores after this point
 
   // Record info about lock acquisition for holding() and debugging.
   lk->cpu = mycpu();
 }
 
-// Release the lock.
+/**
+ * Release the lock.
+ * Panics if the lock is not held.
+ */
 void
 release(struct spinlock *lk)
 {
-  if(!holding(lk))
+  if(!holding(lk)) // If the lock is not held, panic.
     panic("release");
 
-  lk->cpu = 0;
-
-  // Tell the C compiler and the CPU to not move loads or stores
-  // past this point, to ensure that all the stores in the critical
-  // section are visible to other CPUs before the lock is released,
-  // and that loads in the critical section occur strictly before
-  // the lock is released.
-  // On RISC-V, this emits a fence instruction.
-  __sync_synchronize();
-
-  // Release the lock, equivalent to lk->locked = 0.
-  // This code doesn't use a C assignment, since the C standard
-  // implies that an assignment might be implemented with
-  // multiple store instructions.
-  // On RISC-V, sync_lock_release turns into an atomic swap:
-  //   s1 = &lk->locked
-  //   amoswap.w zero, zero, (s1)
-  __sync_lock_release(&lk->locked);
+  lk->cpu = 0;                      // 0 means unheld
+  __sync_synchronize();             // No loads/stores after this point
+  __sync_lock_release(&lk->locked); // Essentially lk->locked = 0
 
   pop_off();
 }

kernel/spinlock.h

@@ -2,7 +2,7 @@
 
 #include "types.h"
 
-// Mutual exclusion lock.
+/** Mutual exclusion spin lock */
 struct spinlock {
   u32 locked; // Is the lock held?
 

user/umalloc.c

@@ -1,7 +1,5 @@
 #include "kernel/types.h"
-#include "kernel/stat.h"
 #include "user/user.h"
-#include "kernel/param.h"
 
 // Memory allocator by Kernighan and Ritchie,
 // The C programming Language, 2nd ed.  Section 8.7.

user/usertests.c

@@ -1,10 +1,8 @@
 #include "kernel/param.h"
 #include "kernel/types.h"
-#include "kernel/stat.h"
 #include "user/user.h"
 #include "kernel/fs.h"
 #include "kernel/fcntl.h"
-#include "kernel/syscall.h"
 #include "kernel/memlayout.h"
 #include "kernel/riscv.h"
 

user/wc.c

@@ -1,5 +1,3 @@
-#include "kernel/types.h"
-#include "kernel/stat.h"
 #include "user/user.h"
 
 char buf[512];