have kill() lock before looking at p->pid

document wait()'s use of np->parent w/o holding lock.

kernel/proc.c

@@ -367,8 +367,12 @@ wait(void)
     // Scan through table looking for exited children.
     havekids = 0;
     for(np = proc; np < &proc[NPROC]; np++){
-      if(np->parent != p)
-        continue;
+      // this code uses np->parent without holding np->lock.
+      // acquiring the lock first would cause a deadlock,
+      // since np might be an ancestor, and we already hold p->lock.
+      if(np->parent == p){
+        // np->parent can't change here because only the parent
+        // changes it, and we're the parent.
         acquire(&np->lock);
         havekids = 1;
         if(np->state == ZOMBIE){
@@ -381,6 +385,7 @@ wait(void)
         }
         release(&np->lock);
       }
+    }
 
     // No point waiting if we don't have any children.
     if(!havekids || p->killed){
@@ -397,8 +402,8 @@ wait(void)
 // Per-CPU process scheduler.
 // Each CPU calls scheduler() after setting itself up.
 // Scheduler never returns.  It loops, doing:
-//  - choose a process to run
-//  - swtch to start running that process
+//  - choose a process to run.
+//  - swtch to start running that process.
 //  - eventually that process transfers control
 //    via swtch back to the scheduler.
 void
@@ -409,7 +414,7 @@ scheduler(void)
   
   c->proc = 0;
   for(;;){
-    // Enable interrupts on this processor.
+    // Give devices a brief chance to interrupt.
     intr_on();
 
     for(p = proc; p < &proc[NPROC]; p++) {
@@ -508,7 +513,7 @@ sleep(void *chan, struct spinlock *lk)
   // change p->state and then call sched.
   // Once we hold p->lock, we can be
   // guaranteed that we won't miss any wakeup
-  // (wakeup runs with p->lock locked),
+  // (wakeup locks p->lock),
   // so it's okay to release lk.
   if(lk != &p->lock){  //DOC: sleeplock0
     acquire(&p->lock);  //DOC: sleeplock1
@@ -559,24 +564,24 @@ wakeup(void *chan)
 
 // Kill the process with the given pid.
 // Process won't exit until it returns
-// to user space (see trap in trap.c).
+// to user space (see usertrap() in trap.c).
 int
 kill(int pid)
 {
   struct proc *p;
 
   for(p = proc; p < &proc[NPROC]; p++){
-    if(p->pid == pid){
     acquire(&p->lock);
-      if(p->pid != pid)
-        panic("kill");
+    if(p->pid == pid){
       p->killed = 1;
-      // Wake process from sleep if necessary.
-      if(p->state == SLEEPING)
+      if(p->state == SLEEPING){
+        // Wake process from sleep().
         p->state = RUNNABLE;
+      }
       release(&p->lock);
       return 0;
     }
+    release(&p->lock);
   }
   return -1;
 }

kernel/syscall.c

@@ -163,8 +163,8 @@ static uint64 (*syscalls[])(void) = {
 [SYS_close]   sys_close,
 };
 
-static void
-dosyscall(void)
+void
+syscall(void)
 {
   int num;
   struct proc *p = myproc();
@@ -180,15 +180,3 @@ dosyscall(void)
     p->tf->a0 = -1;
   }
 }
-
-void
-syscall()
-{
-    if(myproc()->killed)
-      exit();
-    dosyscall();
-    if(myproc()->killed)
-      exit();
-    return;
-}
-

kernel/trap.c

@@ -53,6 +53,9 @@ usertrap(void)
   if(r_scause() == 8){
     // system call
 
+    if(p->killed)
+      exit();
+
     // sepc points to the ecall instruction,
     // but we want to return to the next instruction.
     p->tf->epc += 4;