From 535ac52efadc5c5cdb0483ad55c306cfaff71d50 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Mon, 1 Jul 2019 14:15:18 -0400
Subject: [PATCH 01/19] oops, don't hold mycpu() result across intr_off()

---
 kernel/spinlock.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index 557a86c..5a44a46 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -87,13 +87,12 @@ holding(struct spinlock *lk)
 void
 push_off(void)
 {
-  struct cpu *c = mycpu();
   int old = intr_get();
 
   intr_off();
-  if(c->noff == 0)
-    c->intena = old;
-  c->noff += 1;
+  if(mycpu()->noff == 0)
+    mycpu()->intena = old;
+  mycpu()->noff += 1;
 }
 
 void

From 67702cf706bce7adef472f0caa48d81ddfaeb33a Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 09:14:47 -0400
Subject: [PATCH 02/19] Checkpoint switching to per-process locks, in attempt
 clarify xv6's locking plan, which is a difficult to understand because ptable
 lock protects many invariants.  This implementation has a bug: once in a
 while xv6 unlocks a proc lock that is locked by another core.

---
 kernel/exec.c      |   5 +-
 kernel/fs.c        |   2 +-
 kernel/pipe.c      |   2 +-
 kernel/proc.c      | 194 +++++++++++++++++++++++++++------------------
 kernel/proc.h      |   1 +
 kernel/sleeplock.c |   2 +-
 kernel/spinlock.c  |   8 +-
 kernel/syscall.c   |   3 +
 kernel/sysfile.c   |   2 +-
 kernel/sysproc.c   |   1 +
 kernel/trap.c      |   2 +-
 kernel/uart.c      |   2 +-
 12 files changed, 139 insertions(+), 85 deletions(-)

diff --git a/kernel/exec.c b/kernel/exec.c
index c9af395..b21afbb 100644
--- a/kernel/exec.c
+++ b/kernel/exec.c
@@ -2,6 +2,7 @@
 #include "param.h"
 #include "memlayout.h"
 #include "riscv.h"
+#include "spinlock.h"
 #include "proc.h"
 #include "defs.h"
 #include "elf.h"
@@ -19,7 +20,6 @@ exec(char *path, char **argv)
   struct proghdr ph;
   pagetable_t pagetable = 0, oldpagetable;
   struct proc *p = myproc();
-  uint64 oldsz = p->sz;
 
   begin_op();
 
@@ -60,6 +60,9 @@ exec(char *path, char **argv)
   end_op();
   ip = 0;
 
+  p = myproc();
+  uint64 oldsz = p->sz;
+
   // Allocate two pages at the next page boundary.
   // Use the second as the user stack.
   sz = PGROUNDUP(sz);
diff --git a/kernel/fs.c b/kernel/fs.c
index ebd8f7a..c241b3c 100644
--- a/kernel/fs.c
+++ b/kernel/fs.c
@@ -14,8 +14,8 @@
 #include "defs.h"
 #include "param.h"
 #include "stat.h"
-#include "proc.h"
 #include "spinlock.h"
+#include "proc.h"
 #include "sleeplock.h"
 #include "fs.h"
 #include "buf.h"
diff --git a/kernel/pipe.c b/kernel/pipe.c
index 31bf0cc..eca5959 100644
--- a/kernel/pipe.c
+++ b/kernel/pipe.c
@@ -2,9 +2,9 @@
 #include "riscv.h"
 #include "defs.h"
 #include "param.h"
+#include "spinlock.h"
 #include "proc.h"
 #include "fs.h"
-#include "spinlock.h"
 #include "sleeplock.h"
 #include "file.h"
 
diff --git a/kernel/proc.c b/kernel/proc.c
index 4ae34c8..ae35105 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -2,8 +2,8 @@
 #include "param.h"
 #include "memlayout.h"
 #include "riscv.h"
-#include "proc.h"
 #include "spinlock.h"
+#include "proc.h"
 #include "defs.h"
 
 struct {
@@ -21,7 +21,7 @@ extern void forkret(void);
 // for returning  out of the kernel
 extern void sysexit(void);
 
-static void wakeup1(void *chan);
+static void wakeup1(struct proc *chan);
 
 extern char trampout[]; // trampoline.S
 
@@ -80,11 +80,13 @@ allocproc(void)
   return 0;
 
 found:
+  initlock(&p->lock, "proc");
+  acquire(&p->lock);
+  release(&ptable.lock);
+  
   p->state = EMBRYO;
   p->pid = nextpid++;
 
-  release(&ptable.lock);
-
   // Allocate a page for the kernel stack.
   if((p->kstack = kalloc()) == 0){
     p->state = UNUSED;
@@ -177,15 +179,9 @@ userinit(void)
   safestrcpy(p->name, "initcode", sizeof(p->name));
   p->cwd = namei("/");
 
-  // this assignment to p->state lets other cores
-  // run this process. the acquire forces the above
-  // writes to be visible, and the lock is also needed
-  // because the assignment might not be atomic.
-  acquire(&ptable.lock);
-
   p->state = RUNNABLE;
 
-  release(&ptable.lock);
+  release(&p->lock);
 }
 
 // Grow current process's memory by n bytes.
@@ -196,15 +192,22 @@ growproc(int n)
   uint sz;
   struct proc *p = myproc();
 
+  acquire(&p->lock);
+
   sz = p->sz;
   if(n > 0){
-    if((sz = uvmalloc(p->pagetable, sz, sz + n)) == 0)
+    if((sz = uvmalloc(p->pagetable, sz, sz + n)) == 0) {
+      release(&p->lock);
       return -1;
+    }
   } else if(n < 0){
-    if((sz = uvmdealloc(p->pagetable, sz, sz + n)) == 0)
+    if((sz = uvmdealloc(p->pagetable, sz, sz + n)) == 0) {
+      release(&p->lock);
       return -1;
+    }
   }
   p->sz = sz;
+  release(&p->lock);
   return 0;
 }
 
@@ -244,11 +247,9 @@ fork(void)
 
   pid = np->pid;
 
-  acquire(&ptable.lock);
-
   np->state = RUNNABLE;
 
-  release(&ptable.lock);
+  release(&np->lock);
 
   return pid;
 }
@@ -260,45 +261,65 @@ void
 exit(void)
 {
   struct proc *p = myproc();
-  struct proc *pp;
   int fd;
 
   if(p == initproc)
     panic("init exiting");
 
+  acquire(&p->lock);
+  
   // Close all open files.
   for(fd = 0; fd < NOFILE; fd++){
     if(p->ofile[fd]){
-      fileclose(p->ofile[fd]);
+      struct file *f = p->ofile[fd];
+      release(&p->lock);
+      
+      fileclose(f);
+      
+      acquire(&p->lock);
       p->ofile[fd] = 0;
     }
   }
 
+  struct inode *cwd = p->cwd;
+  release(&p->lock);
+  
   begin_op();
-  iput(p->cwd);
+  iput(cwd);
   end_op();
+
+  acquire(&p->lock);
   p->cwd = 0;
 
-  acquire(&ptable.lock);
-
-  // Parent might be sleeping in wait().
-  wakeup1(p->parent);
-
-  // Pass abandoned children to init.
-  for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
-    if(pp->parent == p){
-      pp->parent = initproc;
-      if(pp->state == ZOMBIE)
-        wakeup1(initproc);
-    }
-  }
-
   // Jump into the scheduler, never to return.
   p->state = ZOMBIE;
   sched();
   panic("zombie exit");
 }
 
+void tellparent(struct proc *p, struct proc *parent) {
+  struct proc *pp;
+
+  acquire(&parent->lock);
+  
+  // Parent might be sleeping in wait().
+  wakeup1(parent);
+
+  // Pass abandoned children to init.
+  for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
+    if(pp->parent == p){
+      pp->parent = initproc;
+      if(pp->state == ZOMBIE) {
+          acquire(&initproc->lock);
+          wakeup1(initproc);
+          acquire(&initproc->lock);
+      }
+    }
+  }
+
+  release(&parent->lock);
+}
+
 // Wait for a child process to exit and return its pid.
 // Return -1 if this process has no children.
 int
@@ -307,14 +328,17 @@ wait(void)
   struct proc *np;
   int havekids, pid;
   struct proc *p = myproc();
+
+  // should p lock!
   
-  acquire(&ptable.lock);
+  acquire(&p->lock);
   for(;;){
     // Scan through table looking for exited children.
     havekids = 0;
     for(np = ptable.proc; np < &ptable.proc[NPROC]; np++){
       if(np->parent != p)
         continue;
+      acquire(&np->lock);
       havekids = 1;
       if(np->state == ZOMBIE){
         // Found one.
@@ -330,19 +354,21 @@ wait(void)
         np->name[0] = 0;
         np->killed = 0;
         np->state = UNUSED;
-        release(&ptable.lock);
+        release(&np->lock);
+        release(&p->lock);
         return pid;
       }
+      release(&np->lock);
     }
 
     // No point waiting if we don't have any children.
     if(!havekids || p->killed){
-      release(&ptable.lock);
+      release(&p->lock);
       return -1;
     }
-
-    // Wait for children to exit.  (See wakeup1 call in proc_exit.)
-    sleep(p, &ptable.lock);  //DOC: wait-sleep
+    
+    // Wait for children to exit.  (See wakeup1 call in tellparent.)
+    sleep(p, &p->lock);  //DOC: wait-sleep
   }
 }
 
@@ -366,13 +392,16 @@ scheduler(void)
     intr_on();
 
     // Loop over process table looking for process to run.
-    acquire(&ptable.lock);
-    for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
-      if(p->state != RUNNABLE)
+    for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+      acquire(&p->lock);
+      
+      if(p->state != RUNNABLE) {
+        release(&p->lock);
         continue;
+      }
 
       // Switch to chosen process.  It is the process's job
-      // to release ptable.lock and then reacquire it
+      // to release its lock and then reacquire it
       // before jumping back to us.
       c->proc = p;
       p->state = RUNNING;
@@ -382,12 +411,17 @@ scheduler(void)
       // Process is done running for now.
       // It should have changed its p->state before coming back.
       c->proc = 0;
+      release(&p->lock);
+
+      if(p->state == ZOMBIE) {
+        tellparent(p, p->parent);
+      }
+
     }
-    release(&ptable.lock);
   }
 }
 
-// Enter scheduler.  Must hold only ptable.lock
+// Enter scheduler.  Must hold only p->lock
 // and have changed proc->state. Saves and restores
 // intena because intena is a property of this
 // kernel thread, not this CPU. It should
@@ -400,8 +434,8 @@ sched(void)
   int intena;
   struct proc *p = myproc();
 
-  if(!holding(&ptable.lock))
-    panic("sched ptable.lock");
+  if(!holding(&p->lock))
+    panic("sched p->lock");
   if(mycpu()->noff != 1)
     panic("sched locks");
   if(p->state == RUNNING)
@@ -418,10 +452,10 @@ sched(void)
 void
 yield(void)
 {
-  acquire(&ptable.lock);  //DOC: yieldlock
+  acquire(&myproc()->lock);  //DOC: yieldlock
   myproc()->state = RUNNABLE;
   sched();
-  release(&ptable.lock);
+  release(&myproc()->lock);
 }
 
 // A fork child's very first scheduling by scheduler()
@@ -431,8 +465,8 @@ forkret(void)
 {
   static int first = 1;
 
-  // Still holding ptable.lock from scheduler.
-  release(&ptable.lock);
+  // Still holding p->lock from scheduler.
+  release(&myproc()->lock);
 
   if (first) {
     // Some initialization functions must be run in the context
@@ -451,60 +485,69 @@ forkret(void)
 void
 sleep(void *chan, struct spinlock *lk)
 {
-  struct proc *p = myproc();
-  
-  if(p == 0)
+  if(myproc() == 0)
     panic("sleep");
 
   if(lk == 0)
     panic("sleep without lk");
 
-  // Must acquire ptable.lock in order to
+  // Must acquire p->lock in order to
   // change p->state and then call sched.
-  // Once we hold ptable.lock, we can be
+  // Once we hold p->lock, we can be
   // guaranteed that we won't miss any wakeup
-  // (wakeup runs with ptable.lock locked),
+  // (wakeup runs with p->lock locked),
   // so it's okay to release lk.
-  if(lk != &ptable.lock){  //DOC: sleeplock0
-    acquire(&ptable.lock);  //DOC: sleeplock1
+  if(lk != &myproc()->lock){  //DOC: sleeplock0
+    acquire(&myproc()->lock);  //DOC: sleeplock1
     release(lk);
   }
   // Go to sleep.
-  p->chan = chan;
-  p->state = SLEEPING;
+  myproc()->chan = chan;
+  myproc()->state = SLEEPING;
 
   sched();
 
   // Tidy up.
-  p->chan = 0;
+  myproc()->chan = 0;
 
   // Reacquire original lock.
-  if(lk != &ptable.lock){  //DOC: sleeplock2
-    release(&ptable.lock);
+  if(lk != &myproc()->lock){  //DOC: sleeplock2
+    release(&myproc()->lock);
     acquire(lk);
   }
 }
 
 //PAGEBREAK!
-// Wake up all processes sleeping on chan.
-// The ptable lock must be held.
+// Wake up all processes sleeping on chan,
+// where chan is a proc.
 static void
-wakeup1(void *chan)
+wakeup1(struct proc *chan)
 {
   struct proc *p;
 
   for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
-    if(p->state == SLEEPING && p->chan == chan)
+    if(p == chan && p->state == SLEEPING && p->chan == chan) {
+      if(p->state != SLEEPING || p->chan != chan)
+        panic("wakeup1");
       p->state = RUNNABLE;
+    }
 }
 
-// Wake up all processes sleeping on chan.
+// Wake up all processes sleeping on chan. Never
+// called when holding a p->lock
 void
 wakeup(void *chan)
 {
-  acquire(&ptable.lock);
-  wakeup1(chan);
-  release(&ptable.lock);
+  struct proc *p;
+
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
+    if(p->state == SLEEPING && p->chan == chan) {
+      acquire(&p->lock);
+      if(p->state != SLEEPING || p->chan != chan)
+        panic("wakeup");
+      p->state = RUNNABLE;
+      release(&p->lock);
+    }
 }
 
 // Kill the process with the given pid.
@@ -515,18 +558,19 @@ kill(int pid)
 {
   struct proc *p;
 
-  acquire(&ptable.lock);
   for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
     if(p->pid == pid){
+      acquire(&p->lock);
+      if(p->pid != pid)
+        panic("kill");
       p->killed = 1;
       // Wake process from sleep if necessary.
       if(p->state == SLEEPING)
         p->state = RUNNABLE;
-      release(&ptable.lock);
+      release(&p->lock);
       return 0;
     }
   }
-  release(&ptable.lock);
   return -1;
 }
 
diff --git a/kernel/proc.h b/kernel/proc.h
index 278e4cd..0fa61ff 100644
--- a/kernel/proc.h
+++ b/kernel/proc.h
@@ -82,6 +82,7 @@ enum procstate { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
 
 // Per-process state
 struct proc {
+  struct spinlock lock;
   char *kstack;                // Bottom of kernel stack for this process
   uint64 sz;                   // Size of process memory (bytes)
   pagetable_t pagetable;       // Page table
diff --git a/kernel/sleeplock.c b/kernel/sleeplock.c
index b490370..81de585 100644
--- a/kernel/sleeplock.c
+++ b/kernel/sleeplock.c
@@ -5,8 +5,8 @@
 #include "defs.h"
 #include "param.h"
 #include "memlayout.h"
-#include "proc.h"
 #include "spinlock.h"
+#include "proc.h"
 #include "sleeplock.h"
 
 void
diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index 5a44a46..c238091 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -20,7 +20,7 @@ initlock(struct spinlock *lk, char *name)
 // Loops (spins) until the lock is acquired.
 // Holding a lock for a long time may cause
 // other CPUs to waste time spinning to acquire it.
-void
+void //__attribute__ ((noinline))
 acquire(struct spinlock *lk)
 {
   push_off(); // disable interrupts to avoid deadlock.
@@ -44,11 +44,13 @@ acquire(struct spinlock *lk)
 }
 
 // Release the lock.
-void
+void //__attribute__ ((noinline))
 release(struct spinlock *lk)
 {
-  if(!holding(lk))
+  if(!holding(lk)) {
+    printf("%p: !holding %s %p\n", mycpu(), lk->name, lk->cpu);
     panic("release");
+  }
 
   lk->cpu = 0;
 
diff --git a/kernel/syscall.c b/kernel/syscall.c
index ca34f2c..8e9d51c 100644
--- a/kernel/syscall.c
+++ b/kernel/syscall.c
@@ -2,6 +2,7 @@
 #include "param.h"
 #include "memlayout.h"
 #include "riscv.h"
+#include "spinlock.h"
 #include "proc.h"
 #include "syscall.h"
 #include "defs.h"
@@ -170,7 +171,9 @@ dosyscall(void)
 
   num = p->tf->a7;
   if(num > 0 && num < NELEM(syscalls) && syscalls[num]) {
+    //printf("%d: syscall %d\n", p->pid, num);
     p->tf->a0 = syscalls[num]();
+    //printf("%d: syscall %d -> %d\n", p->pid, num, p->tf->a0);
   } else {
     printf("%d %s: unknown sys call %d\n",
             p->pid, p->name, num);
diff --git a/kernel/sysfile.c b/kernel/sysfile.c
index 7788de3..33f37f2 100644
--- a/kernel/sysfile.c
+++ b/kernel/sysfile.c
@@ -9,9 +9,9 @@
 #include "defs.h"
 #include "param.h"
 #include "stat.h"
+#include "spinlock.h"
 #include "proc.h"
 #include "fs.h"
-#include "spinlock.h"
 #include "sleeplock.h"
 #include "file.h"
 #include "fcntl.h"
diff --git a/kernel/sysproc.c b/kernel/sysproc.c
index e57e045..65dde26 100644
--- a/kernel/sysproc.c
+++ b/kernel/sysproc.c
@@ -4,6 +4,7 @@
 #include "date.h"
 #include "param.h"
 #include "memlayout.h"
+#include "spinlock.h"
 #include "proc.h"
 
 int
diff --git a/kernel/trap.c b/kernel/trap.c
index 586f123..6c0d04b 100644
--- a/kernel/trap.c
+++ b/kernel/trap.c
@@ -2,8 +2,8 @@
 #include "param.h"
 #include "memlayout.h"
 #include "riscv.h"
-#include "proc.h"
 #include "spinlock.h"
+#include "proc.h"
 #include "defs.h"
 
 struct spinlock tickslock;
diff --git a/kernel/uart.c b/kernel/uart.c
index 35fac1b..b8dd664 100644
--- a/kernel/uart.c
+++ b/kernel/uart.c
@@ -2,8 +2,8 @@
 #include "param.h"
 #include "memlayout.h"
 #include "riscv.h"
-#include "proc.h"
 #include "spinlock.h"
+#include "proc.h"
 #include "defs.h"
 
 //

From da51735980e500922bc108a3444b64ac9450032e Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 13:40:33 -0400
Subject: [PATCH 03/19] Avoid two cores selecting the same process to run

---
 Makefile          |  2 +-
 kernel/proc.c     | 59 ++++++++++++++++++++++++++---------------------
 kernel/riscv.h    |  9 ++++++++
 kernel/spinlock.c | 11 ++++++---
 kernel/spinlock.h |  2 ++
 5 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/Makefile b/Makefile
index 88130e1..9090680 100644
--- a/Makefile
+++ b/Makefile
@@ -54,7 +54,7 @@ OBJCOPY = $(TOOLPREFIX)objcopy
 OBJDUMP = $(TOOLPREFIX)objdump
 
 # CFLAGS = -fno-pic -static -fno-builtin -fno-strict-aliasing -Wall -MD -ggdb -Werror -fno-omit-frame-pointer -O
-CFLAGS = -Wall -Werror -O -fno-omit-frame-pointer -ggdb
+CFLAGS = -Wall -Werror -O0 -fno-omit-frame-pointer -ggdb
 CFLAGS += -mcmodel=medany
 CFLAGS += -ffreestanding -fno-common -nostdlib -mno-relax
 CFLAGS += -I.
diff --git a/kernel/proc.c b/kernel/proc.c
index ae35105..b5d929d 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -297,15 +297,16 @@ exit(void)
   panic("zombie exit");
 }
 
-void tellparent(struct proc *p, struct proc *parent) {
+void reparent(struct proc *p) {
   struct proc *pp;
+  struct proc *parent = p->parent;
 
   acquire(&parent->lock);
   
   // Parent might be sleeping in wait().
   wakeup1(parent);
 
-  // Pass abandoned children to init.
+  // Pass p's abandoned children to init.
   for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
     if(pp->parent == p){
       pp->parent = initproc;
@@ -329,8 +330,6 @@ wait(void)
   int havekids, pid;
   struct proc *p = myproc();
 
-  // should p lock!
-  
   acquire(&p->lock);
   for(;;){
     // Scan through table looking for exited children.
@@ -367,11 +366,27 @@ wait(void)
       return -1;
     }
     
-    // Wait for children to exit.  (See wakeup1 call in tellparent.)
+    // Wait for children to exit.  (See wakeup1 call in reparent.)
     sleep(p, &p->lock);  //DOC: wait-sleep
   }
 }
 
+// Loop over process table looking for process to run.
+struct proc *find_runnable() {
+  struct proc *p;
+  acquire(&ptable.lock);
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+    acquire(&p->lock);
+    if(p->state == RUNNABLE) {
+      release(&ptable.lock);
+      return p;
+    }
+    release(&p->lock);
+  }
+  release(&ptable.lock);
+  return 0;
+}
+  
 //PAGEBREAK: 42
 // Per-CPU process scheduler.
 // Each CPU calls scheduler() after setting itself up.
@@ -391,15 +406,7 @@ scheduler(void)
     // Enable interrupts on this processor.
     intr_on();
 
-    // Loop over process table looking for process to run.
-    for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
-      acquire(&p->lock);
-      
-      if(p->state != RUNNABLE) {
-        release(&p->lock);
-        continue;
-      }
-
+    if((p = find_runnable()) != 0){
       // Switch to chosen process.  It is the process's job
       // to release its lock and then reacquire it
       // before jumping back to us.
@@ -412,11 +419,9 @@ scheduler(void)
       // It should have changed its p->state before coming back.
       c->proc = 0;
       release(&p->lock);
-
       if(p->state == ZOMBIE) {
-        tellparent(p, p->parent);
+        reparent(p);
       }
-
     }
   }
 }
@@ -485,7 +490,9 @@ forkret(void)
 void
 sleep(void *chan, struct spinlock *lk)
 {
-  if(myproc() == 0)
+  struct proc *p = myproc();
+  
+  if(p == 0)
     panic("sleep");
 
   if(lk == 0)
@@ -497,29 +504,29 @@ sleep(void *chan, struct spinlock *lk)
   // guaranteed that we won't miss any wakeup
   // (wakeup runs with p->lock locked),
   // so it's okay to release lk.
-  if(lk != &myproc()->lock){  //DOC: sleeplock0
-    acquire(&myproc()->lock);  //DOC: sleeplock1
+  if(lk != &p->lock){  //DOC: sleeplock0
+    acquire(&p->lock);  //DOC: sleeplock1
     release(lk);
   }
   // Go to sleep.
-  myproc()->chan = chan;
-  myproc()->state = SLEEPING;
+  p->chan = chan;
+  p->state = SLEEPING;
 
   sched();
 
   // Tidy up.
-  myproc()->chan = 0;
+  p->chan = 0;
 
   // Reacquire original lock.
-  if(lk != &myproc()->lock){  //DOC: sleeplock2
-    release(&myproc()->lock);
+  if(lk != &p->lock){  //DOC: sleeplock2
+    release(&p->lock);
     acquire(lk);
   }
 }
 
 //PAGEBREAK!
 // Wake up all processes sleeping on chan,
-// where chan is a proc.
+// where chan is a proc, which is locked.
 static void
 wakeup1(struct proc *chan)
 {
diff --git a/kernel/riscv.h b/kernel/riscv.h
index c3371a4..e5c0f64 100644
--- a/kernel/riscv.h
+++ b/kernel/riscv.h
@@ -304,6 +304,15 @@ w_tp(uint64 x)
   asm volatile("mv tp, %0" : : "r" (x));
 }
 
+static inline uint64
+r_ra()
+{
+  uint64 x;
+  asm volatile("mv %0, ra" : "=r" (x) );
+  return x;
+}
+
+
 #define PGSIZE 4096 // bytes per page
 #define PGSHIFT 12  // bits of offset within a page
 
diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index c238091..1d0b16a 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -20,7 +20,7 @@ initlock(struct spinlock *lk, char *name)
 // Loops (spins) until the lock is acquired.
 // Holding a lock for a long time may cause
 // other CPUs to waste time spinning to acquire it.
-void //__attribute__ ((noinline))
+void
 acquire(struct spinlock *lk)
 {
   push_off(); // disable interrupts to avoid deadlock.
@@ -44,14 +44,19 @@ acquire(struct spinlock *lk)
 }
 
 // Release the lock.
-void //__attribute__ ((noinline))
+void
 release(struct spinlock *lk)
 {
+  uint64 x;
+  asm volatile("mv %0, ra" : "=r" (x) );
   if(!holding(lk)) {
-    printf("%p: !holding %s %p\n", mycpu(), lk->name, lk->cpu);
+    printf("%p: !holding %d %s %p %p %p %p\n", mycpu(), lk->locked, lk->name, lk->cpu,
+           lk->last_release, lk->last_pc, x);
     panic("release");
   }
 
+  lk->last_release = lk->cpu;
+  lk->last_pc  = x;
   lk->cpu = 0;
 
   // Tell the C compiler and the CPU to not move loads or stores
diff --git a/kernel/spinlock.h b/kernel/spinlock.h
index 4392820..5f244c9 100644
--- a/kernel/spinlock.h
+++ b/kernel/spinlock.h
@@ -5,5 +5,7 @@ struct spinlock {
   // For debugging:
   char *name;        // Name of lock.
   struct cpu *cpu;   // The cpu holding the lock.
+  struct cpu *last_release;
+  uint64 last_pc;
 };
 

From 37ac6f8f4fa9a7fc1ddcacc3a97b30c400e52123 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 13:55:52 -0400
Subject: [PATCH 04/19] Don't start processes at the end of the proc table

---
 kernel/proc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index b5d929d..71d4146 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -372,12 +372,14 @@ wait(void)
 }
 
 // Loop over process table looking for process to run.
-struct proc *find_runnable() {
+struct proc *find_runnable(int start) {
   struct proc *p;
   acquire(&ptable.lock);
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+  for(int i = start; i < start+NPROC; i++) {
+    p = &ptable.proc[i % NPROC];
     acquire(&p->lock);
     if(p->state == RUNNABLE) {
+      p->state = RUNNING;
       release(&ptable.lock);
       return p;
     }
@@ -400,19 +402,19 @@ scheduler(void)
 {
   struct proc *p;
   struct cpu *c = mycpu();
-
+  int next;
+  
   c->proc = 0;
   for(;;){
     // Enable interrupts on this processor.
     intr_on();
 
-    if((p = find_runnable()) != 0){
+    if((p = find_runnable(next)) != 0) {
+      next = (next + 1) & NPROC;
       // Switch to chosen process.  It is the process's job
       // to release its lock and then reacquire it
       // before jumping back to us.
       c->proc = p;
-      p->state = RUNNING;
-
       swtch(&c->scheduler, &p->context);
 
       // Process is done running for now.

From 84c759fc02c7843b64a1bafc843cd80fe3c9d7ee Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 14:09:38 -0400
Subject: [PATCH 05/19] x

---
 Makefile      | 2 +-
 kernel/proc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 9090680..88130e1 100644
--- a/Makefile
+++ b/Makefile
@@ -54,7 +54,7 @@ OBJCOPY = $(TOOLPREFIX)objcopy
 OBJDUMP = $(TOOLPREFIX)objdump
 
 # CFLAGS = -fno-pic -static -fno-builtin -fno-strict-aliasing -Wall -MD -ggdb -Werror -fno-omit-frame-pointer -O
-CFLAGS = -Wall -Werror -O0 -fno-omit-frame-pointer -ggdb
+CFLAGS = -Wall -Werror -O -fno-omit-frame-pointer -ggdb
 CFLAGS += -mcmodel=medany
 CFLAGS += -ffreestanding -fno-common -nostdlib -mno-relax
 CFLAGS += -I.
diff --git a/kernel/proc.c b/kernel/proc.c
index 71d4146..c12d97e 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -402,7 +402,7 @@ scheduler(void)
 {
   struct proc *p;
   struct cpu *c = mycpu();
-  int next;
+  int next = 0;
   
   c->proc = 0;
   for(;;){

From 26f306113a1b4057ac1f59050213c8f62c3a211a Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 19:29:14 -0400
Subject: [PATCH 06/19] Fix a lost wakeup bug: the disk driver's wakeup() can
 run after the reading process acquired p->lock and released virtio lock in
 sleep(), but before the process had set p->status to SLEEPING, because the
 wakeup tested p->status without holding p's lock.  Thus, wakeup can complete
 without seeing any process SLEEPING and then p sets p->status to SLEEPING.

Fix some other issues:

- Don't initialize proc lock in allocproc(), because freeproc() sets
np->state = UNUSED and allocproc() can choose np and calls initlock()
on the process's lock, releasing np's lock accidentally.  Move
initializing proc's lock to init.

- Protect nextpid using ptable.lock (and move into its own function)

Some clean up:
- Don't acquire p->lock when it p is used in a private way (e.g., exit()/grow()).
- Move find_runnable() back into scheduler().
---
 kernel/proc.c | 113 ++++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 64 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 8ea09b5..7265174 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -28,7 +28,11 @@ extern char trampout[]; // trampoline.S
 void
 procinit(void)
 {
+  struct proc *p;
+  
   initlock(&ptable.lock, "ptable");
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
+      initlock(&p->lock, "proc");
 }
 
 // Must be called with interrupts disabled,
@@ -60,6 +64,16 @@ myproc(void) {
   return p;
 }
 
+int
+allocpid() {
+  int pid;
+  
+  acquire(&ptable.lock);
+  pid = nextpid++;
+  release(&ptable.lock);
+  return pid;
+}
+
 //PAGEBREAK: 32
 // Look in the process table for an UNUSED proc.
 // If found, change state to EMBRYO and initialize
@@ -70,22 +84,19 @@ allocproc(void)
 {
   struct proc *p;
 
-  acquire(&ptable.lock);
-
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
-    if(p->state == UNUSED)
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+    acquire(&p->lock);
+    if(p->state == UNUSED) {
       goto found;
-
-  release(&ptable.lock);
+    } else {
+      release(&p->lock);
+    }
+  }
   return 0;
 
 found:
-  initlock(&p->lock, "proc");
-  acquire(&p->lock);
-  release(&ptable.lock);
-  
   p->state = EMBRYO;
-  p->pid = nextpid++;
+  p->pid = allocpid();
 
   // Allocate a page for the kernel stack.
   if((p->kstack = kalloc()) == 0){
@@ -217,22 +228,17 @@ growproc(int n)
   uint sz;
   struct proc *p = myproc();
 
-  acquire(&p->lock);
-
   sz = p->sz;
   if(n > 0){
     if((sz = uvmalloc(p->pagetable, sz, sz + n)) == 0) {
-      release(&p->lock);
       return -1;
     }
   } else if(n < 0){
     if((sz = uvmdealloc(p->pagetable, sz, sz + n)) == 0) {
-      release(&p->lock);
       return -1;
     }
   }
   p->sz = sz;
-  release(&p->lock);
   return 0;
 }
 
@@ -294,24 +300,17 @@ exit(void)
   if(p == initproc)
     panic("init exiting");
 
-  acquire(&p->lock);
-  
   // Close all open files.
   for(fd = 0; fd < NOFILE; fd++){
     if(p->ofile[fd]){
       struct file *f = p->ofile[fd];
-      release(&p->lock);
-      
       fileclose(f);
-      
-      acquire(&p->lock);
       p->ofile[fd] = 0;
     }
   }
 
   struct inode *cwd = p->cwd;
-  release(&p->lock);
-  
+
   begin_op();
   iput(cwd);
   end_op();
@@ -389,24 +388,6 @@ wait(void)
   }
 }
 
-// Loop over process table looking for process to run.
-struct proc *find_runnable(int start) {
-  struct proc *p;
-  acquire(&ptable.lock);
-  for(int i = start; i < start+NPROC; i++) {
-    p = &ptable.proc[i % NPROC];
-    acquire(&p->lock);
-    if(p->state == RUNNABLE) {
-      p->state = RUNNING;
-      release(&ptable.lock);
-      return p;
-    }
-    release(&p->lock);
-  }
-  release(&ptable.lock);
-  return 0;
-}
-  
 //PAGEBREAK: 42
 // Per-CPU process scheduler.
 // Each CPU calls scheduler() after setting itself up.
@@ -420,27 +401,31 @@ scheduler(void)
 {
   struct proc *p;
   struct cpu *c = mycpu();
-  int next = 0;
   
   c->proc = 0;
   for(;;){
     // Enable interrupts on this processor.
     intr_on();
 
-    if((p = find_runnable(next)) != 0) {
-      next = (next + 1) & NPROC;
-      // Switch to chosen process.  It is the process's job
-      // to release its lock and then reacquire it
-      // before jumping back to us.
-      c->proc = p;
-      swtch(&c->scheduler, &p->context);
+    for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+      acquire(&p->lock);
+      if(p->state == RUNNABLE) {
+        // Switch to chosen process.  It is the process's job
+        // to release its lock and then reacquire it
+        // before jumping back to us.
+        p->state = RUNNING;
+        c->proc = p;
+        swtch(&c->scheduler, &p->context);
 
-      // Process is done running for now.
-      // It should have changed its p->state before coming back.
-      c->proc = 0;
-      release(&p->lock);
-      if(p->state == ZOMBIE) {
-        reparent(p);
+        // Process is done running for now.
+        // It should have changed its p->state before coming back.
+        c->proc = 0;
+        release(&p->lock);
+        if(p->state == ZOMBIE) {
+          reparent(p);
+        }
+      } else {
+        release(&p->lock);
       }
     }
   }
@@ -477,10 +462,11 @@ sched(void)
 void
 yield(void)
 {
-  acquire(&myproc()->lock);  //DOC: yieldlock
-  myproc()->state = RUNNABLE;
+  struct proc *p = myproc();
+  acquire(&p->lock);  //DOC: yieldlock
+  p->state = RUNNABLE;
   sched();
-  release(&myproc()->lock);
+  release(&p->lock);
 }
 
 // A fork child's very first scheduling by scheduler()
@@ -567,14 +553,13 @@ wakeup(void *chan)
 {
   struct proc *p;
 
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
+  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+    acquire(&p->lock);
     if(p->state == SLEEPING && p->chan == chan) {
-      acquire(&p->lock);
-      if(p->state != SLEEPING || p->chan != chan)
-        panic("wakeup");
       p->state = RUNNABLE;
-      release(&p->lock);
     }
+    release(&p->lock);
+  }
 }
 
 // Kill the process with the given pid.

From ccf299850b51bb6f899765cc41a826d903e9e885 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Tue, 2 Jul 2019 20:52:30 -0400
Subject: [PATCH 07/19] Remove some debugging code

---
 kernel/spinlock.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/spinlock.c b/kernel/spinlock.c
index 1d0b16a..5a44a46 100644
--- a/kernel/spinlock.c
+++ b/kernel/spinlock.c
@@ -47,16 +47,9 @@ acquire(struct spinlock *lk)
 void
 release(struct spinlock *lk)
 {
-  uint64 x;
-  asm volatile("mv %0, ra" : "=r" (x) );
-  if(!holding(lk)) {
-    printf("%p: !holding %d %s %p %p %p %p\n", mycpu(), lk->locked, lk->name, lk->cpu,
-           lk->last_release, lk->last_pc, x);
+  if(!holding(lk))
     panic("release");
-  }
 
-  lk->last_release = lk->cpu;
-  lk->last_pc  = x;
   lk->cpu = 0;
 
   // Tell the C compiler and the CPU to not move loads or stores

From cee830af2484fb3e67f3778e03114179eddc4f82 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Wed, 3 Jul 2019 15:18:55 -0400
Subject: [PATCH 08/19] Apply some corresponding bug fixes from wq branch here

---
 kernel/proc.c | 54 +++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 7265174..9202b61 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -288,6 +288,22 @@ fork(void)
   return pid;
 }
 
+void reparent(struct proc *p) {
+  struct proc *pp;
+
+  // Pass p's abandoned children to init.
+  for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
+    acquire(&pp->lock);
+    if(pp->parent == p){
+      pp->parent = initproc;
+      if(pp->state == ZOMBIE) {
+          wakeup1(initproc);
+      }
+    }
+    release(&pp->lock);
+  }
+}
+
 // Exit the current process.  Does not return.
 // An exited process remains in the zombie state
 // until its parent calls wait().
@@ -315,39 +331,24 @@ exit(void)
   iput(cwd);
   end_op();
 
+  reparent(p);
+  
+  acquire(&p->parent->lock);
+    
   acquire(&p->lock);
   p->cwd = 0;
+  p->state = ZOMBIE;
+
+  release(&p->parent->lock);
+
+  // Parent might be sleeping in wait().
+  wakeup1(p->parent);
 
   // Jump into the scheduler, never to return.
-  p->state = ZOMBIE;
   sched();
   panic("zombie exit");
 }
 
-void reparent(struct proc *p) {
-  struct proc *pp;
-  struct proc *parent = p->parent;
-
-  acquire(&parent->lock);
-  
-  // Parent might be sleeping in wait().
-  wakeup1(parent);
-
-  // Pass p's abandoned children to init.
-  for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
-    if(pp->parent == p){
-      pp->parent = initproc;
-      if(pp->state == ZOMBIE) {
-          acquire(&initproc->lock);
-          wakeup1(initproc);
-          acquire(&initproc->lock);
-      }
-    }
-  }
-
-  release(&parent->lock);
-}
-
 // Wait for a child process to exit and return its pid.
 // Return -1 if this process has no children.
 int
@@ -421,9 +422,6 @@ scheduler(void)
         // It should have changed its p->state before coming back.
         c->proc = 0;
         release(&p->lock);
-        if(p->state == ZOMBIE) {
-          reparent(p);
-        }
       } else {
         release(&p->lock);
       }

From 47e69250d08acc7d91d791148a37f4279f5939c9 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Wed, 3 Jul 2019 15:38:30 -0400
Subject: [PATCH 09/19] Simplify wakeup1

---
 kernel/proc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 9202b61..1201c8b 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -529,19 +529,13 @@ sleep(void *chan, struct spinlock *lk)
 }
 
 //PAGEBREAK!
-// Wake up all processes sleeping on chan,
-// where chan is a proc, which is locked.
+// Wake up locked parent, used by exit()
 static void
-wakeup1(struct proc *chan)
+wakeup1(struct proc *p)
 {
-  struct proc *p;
-
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
-    if(p == chan && p->state == SLEEPING && p->chan == chan) {
-      if(p->state != SLEEPING || p->chan != chan)
-        panic("wakeup1");
-      p->state = RUNNABLE;
-    }
+  if(p->chan == p && p->state == SLEEPING) {
+    p->state = RUNNABLE;
+  }
 }
 
 // Wake up all processes sleeping on chan. Never

From 6bfb078b146c95918752afd3b3308d748666f4ae Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Thu, 4 Jul 2019 08:54:16 -0400
Subject: [PATCH 10/19] x

---
 kernel/proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 1201c8b..9c64c4a 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -288,10 +288,10 @@ fork(void)
   return pid;
 }
 
+// Pass p's abandoned children to init.
 void reparent(struct proc *p) {
   struct proc *pp;
 
-  // Pass p's abandoned children to init.
   for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
     acquire(&pp->lock);
     if(pp->parent == p){

From fab5e7c1de2288e2b9e41f7010ca85f2a641cf63 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Thu, 4 Jul 2019 08:54:23 -0400
Subject: [PATCH 11/19] Make size in stat.h be a uint64 Supporting print long
 using %l (a bit of cheat) Modify ls to print size using %l We should probably
 update size in inode too.

---
 kernel/stat.h | 4 ++--
 user/ls.c     | 2 +-
 user/printf.c | 2 ++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/stat.h b/kernel/stat.h
index a498321..19543af 100644
--- a/kernel/stat.h
+++ b/kernel/stat.h
@@ -3,9 +3,9 @@
 #define T_DEVICE  3   // Device
 
 struct stat {
-  short type;  // Type of file
   int dev;     // File system's disk device
   uint ino;    // Inode number
+  short type;  // Type of file
   short nlink; // Number of links to file
-  uint size;   // Size of file in bytes
+  uint64 size; // Size of file in bytes
 };
diff --git a/user/ls.c b/user/ls.c
index c649c57..3511d87 100644
--- a/user/ls.c
+++ b/user/ls.c
@@ -43,7 +43,7 @@ ls(char *path)
 
   switch(st.type){
   case T_FILE:
-    printf(1, "%s %d %d %d\n", fmtname(path), st.type, st.ino, st.size);
+    printf(1, "%s %d %d %l\n", fmtname(path), st.type, st.ino, st.size);
     break;
 
   case T_DIR:
diff --git a/user/printf.c b/user/printf.c
index 0c6b34b..f3b3282 100644
--- a/user/printf.c
+++ b/user/printf.c
@@ -68,6 +68,8 @@ printf(int fd, const char *fmt, ...)
     } else if(state == '%'){
       if(c == 'd'){
         printint(fd, va_arg(ap, int), 10, 1);
+      } else if(c == 'l') {
+        printint(fd, va_arg(ap, uint64), 10, 0);
       } else if(c == 'x') {
         printint(fd, va_arg(ap, int), 16, 0);
       } else if(c == 'p') {

From be88befed702f7bcbf65212a9dcf9456a7bd2ae1 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Fri, 5 Jul 2019 11:44:51 -0400
Subject: [PATCH 12/19] two exit/exit tests

---
 user/usertests.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/user/usertests.c b/user/usertests.c
index ef70bfb..199346c 100644
--- a/user/usertests.c
+++ b/user/usertests.c
@@ -417,16 +417,18 @@ exitwait(void)
 {
   int i, pid;
 
+  printf(1, "exitwait test\n");
+
   for(i = 0; i < 100; i++){
     pid = fork();
     if(pid < 0){
       printf(1, "fork failed\n");
-      return;
+      exit();
     }
     if(pid){
       if(wait() != pid){
         printf(1, "wait wrong pid\n");
-        return;
+        exit();
       }
     } else {
       exit();
@@ -435,6 +437,75 @@ exitwait(void)
   printf(1, "exitwait ok\n");
 }
 
+// try to find races in the reparenting
+// code that handles a parent exiting
+// when it still has live children.
+void
+reparent(void)
+{
+  int master_pid = getpid();
+  
+  printf(1, "reparent test\n");
+
+  for(int i = 0; i < 100; i++){
+    int pid = fork();
+    if(pid < 0){
+      printf(1, "fork failed\n");
+      exit();
+    }
+    if(pid){
+      if(wait() != pid){
+        printf(1, "wait wrong pid\n");
+        exit();
+      }
+    } else {
+      int pid2 = fork();
+      if(pid2 < 0){
+        printf(1, "fork failed\n");
+        kill(master_pid);
+        exit();
+      }
+      if(pid2 == 0){
+        exit();
+      } else {
+        exit();
+      }
+    }
+  }
+  printf(1, "reparent ok\n");
+}
+
+// what if two children exit() at the same time?
+void
+twochildren(void)
+{
+  printf(1, "twochildren test\n");
+
+  for(int i = 0; i < 1000; i++){
+    int pid1 = fork();
+    if(pid1 < 0){
+      printf(1, "fork failed\n");
+      exit();
+    }
+    if(pid1 == 0){
+      exit();
+    } else {
+      int pid2 = fork();
+      if(pid2 < 0){
+        printf(1, "fork failed\n");
+        exit();
+      }
+      if(pid2 == 0){
+        exit();
+      } else {
+        wait();
+        wait();
+      }
+    }
+  }
+  printf(1, "twochildren ok\n");
+}
+
 void
 mem(void)
 {
@@ -1751,6 +1822,9 @@ main(int argc, char *argv[])
   }
   close(open("usertests.ran", O_CREATE));
 
+  reparent();
+  twochildren();
+  
   argptest();
   createdelete();
   linkunlink();

From dabbc348bc4edab522901d8473acaabe276cdc45 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Sat, 6 Jul 2019 16:38:41 -0400
Subject: [PATCH 13/19] Maybe fix two races identified by rtm (thx!):

- during exit(), hold p's parent lock and p's lock across all changes
to p and its parent (e.g., reparenting and wakeup1).  the lock
ordering between concurrent exits of children, parent, and great
parent might work out because processes form a tree.

- in wakeup1() test and set p->state atomically by asking caller to
have p locked.

a correctness proof would be desirable.
---
 kernel/proc.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 9c64c4a..2266ca6 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -288,22 +288,28 @@ fork(void)
   return pid;
 }
 
-// Pass p's abandoned children to init.
-void reparent(struct proc *p) {
+// Pass p's abandoned children to init. p and p's parent
+// are locked.
+void reparent(struct proc *p, struct proc *parent) {
   struct proc *pp;
 
   for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
-    acquire(&pp->lock);
-    if(pp->parent == p){
-      pp->parent = initproc;
-      if(pp->state == ZOMBIE) {
+    if (pp != p && pp != parent) {
+      acquire(&pp->lock);
+      if(pp->parent == p){
+        pp->parent = initproc;
+        if(pp->state == ZOMBIE) {
+          acquire(&initproc->lock);
           wakeup1(initproc);
+          release(&initproc->lock);
+        }
       }
+      release(&pp->lock);
     }
-    release(&pp->lock);
   }
 }
 
+
 // Exit the current process.  Does not return.
 // An exited process remains in the zombie state
 // until its parent calls wait().
@@ -331,19 +337,20 @@ exit(void)
   iput(cwd);
   end_op();
 
-  reparent(p);
-  
   acquire(&p->parent->lock);
     
   acquire(&p->lock);
+
+  reparent(p, p->parent);
+
   p->cwd = 0;
   p->state = ZOMBIE;
 
-  release(&p->parent->lock);
-
   // Parent might be sleeping in wait().
   wakeup1(p->parent);
 
+  release(&p->parent->lock);
+
   // Jump into the scheduler, never to return.
   sched();
   panic("zombie exit");
@@ -529,7 +536,8 @@ sleep(void *chan, struct spinlock *lk)
 }
 
 //PAGEBREAK!
-// Wake up locked parent, used by exit()
+// Wake up p, used by exit()
+// Caller should lock p.
 static void
 wakeup1(struct proc *p)
 {

From 62313be582a5abd3a114eec3c9f5770ea37ef766 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Sun, 7 Jul 2019 06:39:31 -0400
Subject: [PATCH 14/19] another fork test

---
 user/usertests.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/user/usertests.c b/user/usertests.c
index 199346c..9d46b1a 100644
--- a/user/usertests.c
+++ b/user/usertests.c
@@ -506,6 +506,40 @@ twochildren(void)
   printf(1, "twochildren ok\n");
 }
 
+void
+forkforkfork(void)
+{
+  printf(1, "forkforkfork test\n");
+
+  unlink("stopforking");
+
+  int pid = fork();
+  if(pid < 0){
+    printf(1, "fork failed");
+    exit();
+  }
+  if(pid == 0){
+    while(1){
+      int fd = open("stopforking", 0);
+      if(fd >= 0){
+        exit();
+      }
+      if(fork() < 0){
+        close(open("stopforking", O_CREATE|O_RDWR));
+      }
+    }
+
+    exit();
+  }
+
+  sleep(2);
+  close(open("stopforking", O_CREATE|O_RDWR));
+  wait();
+  sleep(1);
+
+  printf(1, "forkforkfork ok\n");
+}
+
 void
 mem(void)
 {
@@ -1824,6 +1858,7 @@ main(int argc, char *argv[])
 
   reparent();
   twochildren();
+  forkforkfork();
   
   argptest();
   createdelete();

From c4f6a241cdc220dafe01bc7ca2ca7f8a253a838c Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Sun, 7 Jul 2019 07:03:28 -0400
Subject: [PATCH 15/19] avoid a double-lock of initproc->lock if child of init
 is reparenting

---
 kernel/proc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 2266ca6..cfb3262 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -292,6 +292,7 @@ fork(void)
 // are locked.
 void reparent(struct proc *p, struct proc *parent) {
   struct proc *pp;
+  int init_is_my_parent = (p->parent == initproc);
 
   for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
     if (pp != p && pp != parent) {
@@ -299,9 +300,11 @@ void reparent(struct proc *p, struct proc *parent) {
       if(pp->parent == p){
         pp->parent = initproc;
         if(pp->state == ZOMBIE) {
-          acquire(&initproc->lock);
+          if(!init_is_my_parent)
+            acquire(&initproc->lock);
           wakeup1(initproc);
-          release(&initproc->lock);
+          if(!init_is_my_parent)
+            release(&initproc->lock);
         }
       }
       release(&pp->lock);

From 4ce3a5fa21bc1ecda7b833a0f8fb05cdcd6d3a67 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Sun, 7 Jul 2019 14:57:16 -0400
Subject: [PATCH 16/19] nits

---
 kernel/proc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index cfb3262..44bacc2 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -76,8 +76,9 @@ allocpid() {
 
 //PAGEBREAK: 32
 // Look in the process table for an UNUSED proc.
-// If found, change state to EMBRYO and initialize
-// state required to run in the kernel.
+// If found, change state to EMBRYO, initialize
+// state required to run in the kernel,
+// and return with p->lock held.
 // Otherwise return 0.
 static struct proc*
 allocproc(void)
@@ -124,7 +125,7 @@ found:
 
 // free a proc structure and the data hanging from it,
 // including user pages.
-// the proc lock must be held.
+// p->lock must be held.
 static void
 freeproc(struct proc *p)
 {
@@ -259,6 +260,7 @@ fork(void)
   // Copy user memory from parent to child.
   if(uvmcopy(p->pagetable, np->pagetable, p->sz) < 0){
     freeproc(np);
+    release(&np->lock);
     return -1;
   }
   np->sz = p->sz;
@@ -290,9 +292,10 @@ fork(void)
 
 // Pass p's abandoned children to init. p and p's parent
 // are locked.
-void reparent(struct proc *p, struct proc *parent) {
+void
+reparent(struct proc *p, struct proc *parent) {
   struct proc *pp;
-  int init_is_my_parent = (p->parent == initproc);
+  int child_of_init = (p->parent == initproc);
 
   for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
     if (pp != p && pp != parent) {
@@ -300,10 +303,10 @@ void reparent(struct proc *p, struct proc *parent) {
       if(pp->parent == p){
         pp->parent = initproc;
         if(pp->state == ZOMBIE) {
-          if(!init_is_my_parent)
+          if(!child_of_init)
             acquire(&initproc->lock);
           wakeup1(initproc);
-          if(!init_is_my_parent)
+          if(!child_of_init)
             release(&initproc->lock);
         }
       }
@@ -431,10 +434,8 @@ scheduler(void)
         // Process is done running for now.
         // It should have changed its p->state before coming back.
         c->proc = 0;
-        release(&p->lock);
-      } else {
-        release(&p->lock);
       }
+      release(&p->lock);
     }
   }
 }

From db72f3108fb729d4a9bbcc7ed3979a08eeadd022 Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Sun, 7 Jul 2019 15:20:13 -0400
Subject: [PATCH 17/19] eliminate ptable. ptable.lock -> pid_lock.

---
 kernel/proc.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index 44bacc2..ade12d9 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -6,16 +6,15 @@
 #include "proc.h"
 #include "defs.h"
 
-struct {
-  struct spinlock lock;
-  struct proc proc[NPROC];
-} ptable;
+struct proc proc[NPROC];
 
 struct cpu cpus[NCPU];
 
 struct proc *initproc;
 
+struct spinlock pid_lock;
 int nextpid = 1;
+
 extern void forkret(void);
 
 // for returning  out of the kernel
@@ -30,8 +29,8 @@ procinit(void)
 {
   struct proc *p;
   
-  initlock(&ptable.lock, "ptable");
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++)
+  initlock(&pid_lock, "nextpid");
+  for(p = proc; p < &proc[NPROC]; p++)
       initlock(&p->lock, "proc");
 }
 
@@ -68,9 +67,9 @@ int
 allocpid() {
   int pid;
   
-  acquire(&ptable.lock);
+  acquire(&pid_lock);
   pid = nextpid++;
-  release(&ptable.lock);
+  release(&pid_lock);
   return pid;
 }
 
@@ -85,7 +84,7 @@ allocproc(void)
 {
   struct proc *p;
 
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+  for(p = proc; p < &proc[NPROC]; p++) {
     acquire(&p->lock);
     if(p->state == UNUSED) {
       goto found;
@@ -297,7 +296,7 @@ reparent(struct proc *p, struct proc *parent) {
   struct proc *pp;
   int child_of_init = (p->parent == initproc);
 
-  for(pp = ptable.proc; pp < &ptable.proc[NPROC]; pp++){
+  for(pp = proc; pp < &proc[NPROC]; pp++){
     if (pp != p && pp != parent) {
       acquire(&pp->lock);
       if(pp->parent == p){
@@ -375,7 +374,7 @@ wait(void)
   for(;;){
     // Scan through table looking for exited children.
     havekids = 0;
-    for(np = ptable.proc; np < &ptable.proc[NPROC]; np++){
+    for(np = proc; np < &proc[NPROC]; np++){
       if(np->parent != p)
         continue;
       acquire(&np->lock);
@@ -421,7 +420,7 @@ scheduler(void)
     // Enable interrupts on this processor.
     intr_on();
 
-    for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+    for(p = proc; p < &proc[NPROC]; p++) {
       acquire(&p->lock);
       if(p->state == RUNNABLE) {
         // Switch to chosen process.  It is the process's job
@@ -557,7 +556,7 @@ wakeup(void *chan)
 {
   struct proc *p;
 
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++) {
+  for(p = proc; p < &proc[NPROC]; p++) {
     acquire(&p->lock);
     if(p->state == SLEEPING && p->chan == chan) {
       p->state = RUNNABLE;
@@ -574,7 +573,7 @@ kill(int pid)
 {
   struct proc *p;
 
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
+  for(p = proc; p < &proc[NPROC]; p++){
     if(p->pid == pid){
       acquire(&p->lock);
       if(p->pid != pid)
@@ -637,7 +636,7 @@ procdump(void)
   struct proc *p;
   char *state;
 
-  for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
+  for(p = proc; p < &proc[NPROC]; p++){
     if(p->state == UNUSED)
       continue;
     if(p->state >= 0 && p->state < NELEM(states) && states[p->state])

From adcc6129013786e89357c16793eb96c75d687454 Mon Sep 17 00:00:00 2001
From: Frans Kaashoek <kaashoek@mit.edu>
Date: Mon, 8 Jul 2019 08:50:55 -0400
Subject: [PATCH 18/19] Update runoff list for producing xv6.pdf

---
 runoff.list | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/runoff.list b/runoff.list
index 6e6af18..f3e9224 100644
--- a/runoff.list
+++ b/runoff.list
@@ -9,6 +9,7 @@ kernel/date.h
 
 # entering xv6
 kernel/entry.S
+kernel/start.c
 kernel/main.c
 
 # locks
@@ -24,6 +25,7 @@ kernel/kalloc.c
 
 # system calls
 user/usys.pl
+kernel/kernelvec.S
 kernel/trap.c
 kernel/syscall.h
 kernel/syscall.c

From 9d34838b4f7c859b753a32124002d7d845140b0a Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Mon, 8 Jul 2019 11:11:00 -0400
Subject: [PATCH 19/19] holding p->lock all the way through state=RUNNABLE
 means we don't need EMBRYO

---
 kernel/proc.c | 17 ++++++-----------
 kernel/proc.h |  2 +-
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/kernel/proc.c b/kernel/proc.c
index ade12d9..a947f7f 100644
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -75,8 +75,7 @@ allocpid() {
 
 //PAGEBREAK: 32
 // Look in the process table for an UNUSED proc.
-// If found, change state to EMBRYO, initialize
-// state required to run in the kernel,
+// If found, initialize state required to run in the kernel,
 // and return with p->lock held.
 // Otherwise return 0.
 static struct proc*
@@ -95,18 +94,17 @@ allocproc(void)
   return 0;
 
 found:
-  p->state = EMBRYO;
   p->pid = allocpid();
 
   // Allocate a page for the kernel stack.
   if((p->kstack = kalloc()) == 0){
-    p->state = UNUSED;
     return 0;
   }
 
   // Allocate a trapframe page.
   if((p->tf = (struct trapframe *)kalloc()) == 0){
-    p->state = UNUSED;
+    kfree(p->kstack);
+    p->kstack = 0;
     return 0;
   }
 
@@ -208,7 +206,7 @@ userinit(void)
   uvminit(p->pagetable, initcode, sizeof(initcode));
   p->sz = PGSIZE;
 
-  // prepare for the very first kernel->user.
+  // prepare for the very first "return" from kernel to user.
   p->tf->epc = 0;
   p->tf->sp = PGSIZE;
 
@@ -336,11 +334,10 @@ exit(void)
     }
   }
 
-  struct inode *cwd = p->cwd;
-
   begin_op();
-  iput(cwd);
+  iput(p->cwd);
   end_op();
+  p->cwd = 0;
 
   acquire(&p->parent->lock);
     
@@ -348,7 +345,6 @@ exit(void)
 
   reparent(p, p->parent);
 
-  p->cwd = 0;
   p->state = ZOMBIE;
 
   // Parent might be sleeping in wait().
@@ -627,7 +623,6 @@ procdump(void)
 {
   static char *states[] = {
   [UNUSED]    "unused",
-  [EMBRYO]    "embryo",
   [SLEEPING]  "sleep ",
   [RUNNABLE]  "runble",
   [RUNNING]   "run   ",
diff --git a/kernel/proc.h b/kernel/proc.h
index 0fa61ff..687fdd1 100644
--- a/kernel/proc.h
+++ b/kernel/proc.h
@@ -78,7 +78,7 @@ struct trapframe {
   /* 280 */ uint64 t6;
 };
 
-enum procstate { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
+enum procstate { UNUSED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
 
 // Per-process state
 struct proc {