more doc tweaks

Author: rsc

TRICKS

@@ -4,6 +4,9 @@ might be worth pointing out in a longer explanation or in class.
 
 ---
 
+[2009/07/12: No longer relevant; forkret1 changed
+and this is now cleaner.]
+
 forkret1 in trapasm.S is called with a tf argument.
 In order to use it, forkret1 copies the tf pointer into
 %esp and then jumps to trapret, which pops the 
@@ -45,21 +48,21 @@ always.
 
 There is a (harmless) race in pushcli, which does
 
-	eflags = read_eflags();
+	eflags = readeflags();
 	cli();
-	if(cpus[cpu()].ncli++ == 0)
-		cpus[cpu()].intena = eflags & FL_IF;
+	if(c->ncli++ == 0)
+		c->intena = eflags & FL_IF;
 
 Consider a bottom-level pushcli.  
 If interrupts are disabled already, then the right thing
 happens: read_eflags finds that FL_IF is not set,
-and intena = 1.  If interrupts are enabled, then
+and intena = 0.  If interrupts are enabled, then
 it is less clear that the right thing happens:
-the read_eflags can execute, then the process
+the readeflags can execute, then the process
 can get preempted and rescheduled on another cpu,
 and then once it starts running, perhaps with 
 interrupts disabled (can happen since the scheduler
-only disables interrupts once per scheduling loop,
+only enables interrupts once per scheduling loop,
 not every time it schedules a process), it will 
 incorrectly record that interrupts *were* enabled.
 This doesn't matter, because if it was safe to be
@@ -112,17 +115,13 @@ processors will need it.
 
 ---
 
-The code in sys_fork needs to read np->pid before
+The code in fork needs to read np->pid before
 setting np->state to RUNNABLE.  
 
 	int
-	sys_fork(void)
+	fork(void)
 	{
-	  int pid;
-	  struct proc *np;
-	
-	  if((np = copyproc(cp)) == 0)
-	    return -1;
+	  ...
 	  pid = np->pid;
 	  np->state = RUNNABLE;
 	  return pid;
@@ -134,3 +133,4 @@ get reused for a different process (with a new pid), all
 before the return statement.  So it's not safe to just do
 "return np->pid;".
 
+This works because proc.h marks the pid as volatile.

pipe.c

@@ -9,12 +9,12 @@
 #define PIPESIZE 512
 
 struct pipe {
-  int readopen;   // read fd is still open
-  int writeopen;  // write fd is still open
-  uint writep;    // next index to write
-  uint readp;     // next index to read
   struct spinlock lock;
   char data[PIPESIZE];
+  uint nread;     // number of bytes read
+  uint nwrite;    // number of bytes written
+  int readopen;   // read fd is still open
+  int writeopen;  // write fd is still open
 };
 
 int
@@ -30,8 +30,8 @@ pipealloc(struct file **f0, struct file **f1)
     goto bad;
   p->readopen = 1;
   p->writeopen = 1;
-  p->writep = 0;
-  p->readp = 0;
+  p->nwrite = 0;
+  p->nread = 0;
   initlock(&p->lock, "pipe");
   (*f0)->type = FD_PIPE;
   (*f0)->readable = 1;
@@ -60,10 +60,10 @@ pipeclose(struct pipe *p, int writable)
   acquire(&p->lock);
   if(writable){
     p->writeopen = 0;
-    wakeup(&p->readp);
+    wakeup(&p->nread);
   } else {
     p->readopen = 0;
-    wakeup(&p->writep);
+    wakeup(&p->nwrite);
   }
   if(p->readopen == 0 && p->writeopen == 0) {
     release(&p->lock);
@@ -80,19 +80,19 @@ pipewrite(struct pipe *p, char *addr, int n)
 
   acquire(&p->lock);
   for(i = 0; i < n; i++){
-    while(p->writep == p->readp + PIPESIZE) {
+    while(p->nwrite == p->nread + PIPESIZE) {  //DOC: pipewrite-full
       if(p->readopen == 0 || cp->killed){
         release(&p->lock);
         return -1;
       }
-      wakeup(&p->readp);
-      sleep(&p->writep, &p->lock);
+      wakeup(&p->nread);
+      sleep(&p->nwrite, &p->lock);  //DOC: pipewrite-sleep
     }
-    p->data[p->writep++ % PIPESIZE] = addr[i];
+    p->data[p->nwrite++ % PIPESIZE] = addr[i];
   }
-  wakeup(&p->readp);
+  wakeup(&p->nread);  //DOC: pipewrite-wakeup1
   release(&p->lock);
-  return i;
+  return n;
 }
 
 int
@@ -101,19 +101,19 @@ piperead(struct pipe *p, char *addr, int n)
   int i;
 
   acquire(&p->lock);
-  while(p->readp == p->writep && p->writeopen){
+  while(p->nread == p->nwrite && p->writeopen){  //DOC: pipe-empty
     if(cp->killed){
       release(&p->lock);
       return -1;
     }
-    sleep(&p->readp, &p->lock);
+    sleep(&p->nread, &p->lock); //DOC: piperead-sleep
   }
-  for(i = 0; i < n; i++){
-    if(p->readp == p->writep)
+  for(i = 0; i < n; i++){  //DOC: piperead-copy
+    if(p->nread == p->nwrite)
       break;
-    addr[i] = p->data[p->readp++ % PIPESIZE];
+    addr[i] = p->data[p->nread++ % PIPESIZE];
   }
-  wakeup(&p->writep);
+  wakeup(&p->nwrite);  //DOC: piperead-wakeup
   release(&p->lock);
   return i;
 }

proc.c

@@ -290,8 +290,8 @@ sleep(void *chan, struct spinlock *lk)
   // guaranteed that we won't miss any wakeup
   // (wakeup runs with ptable.lock locked),
   // so it's okay to release lk.
-  if(lk != &ptable.lock){
-    acquire(&ptable.lock);
+  if(lk != &ptable.lock){  //DOC: sleeplock0
+    acquire(&ptable.lock);  //DOC: sleeplock1
     release(lk);
   }
 
@@ -304,7 +304,7 @@ sleep(void *chan, struct spinlock *lk)
   cp->chan = 0;
 
   // Reacquire original lock.
-  if(lk != &ptable.lock){
+  if(lk != &ptable.lock){  //DOC: sleeplock2
     release(&ptable.lock);
     acquire(lk);
   }
@@ -393,7 +393,6 @@ exit(void)
   }
 
   // Jump into the scheduler, never to return.
-  cp->killed = 0;
   cp->state = ZOMBIE;
   sched();
   panic("zombie exit");
@@ -412,22 +411,21 @@ wait(void)
     // Scan through table looking for zombie children.
     havekids = 0;
     for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){
-      if(p->state == UNUSED)
+      if(p->parent != cp)
         continue;
-      if(p->parent == cp){
-        havekids = 1;
-        if(p->state == ZOMBIE){
-          // Found one.
-          kfree(p->mem, p->sz);
-          kfree(p->kstack, KSTACKSIZE);
-          pid = p->pid;
-          p->state = UNUSED;
-          p->pid = 0;
-          p->parent = 0;
-          p->name[0] = 0;
-          release(&ptable.lock);
-          return pid;
-        }
+      havekids = 1;
+      if(p->state == ZOMBIE){
+        // Found one.
+        pid = p->pid;
+        kfree(p->mem, p->sz);
+        kfree(p->kstack, KSTACKSIZE);
+        p->state = UNUSED;
+        p->pid = 0;
+        p->parent = 0;
+        p->name[0] = 0;
+        p->killed = 0;
+        release(&ptable.lock);
+        return pid;
       }
     }
 
@@ -438,7 +436,7 @@ wait(void)
     }
 
     // Wait for children to exit.  (See wakeup1 call in proc_exit.)
-    sleep(cp, &ptable.lock);
+    sleep(cp, &ptable.lock);  //DOC: wait-sleep
   }
 }
 

web/l-coordination.html

@@ -47,7 +47,7 @@ <h2>xv6 code examples</h2>
 <h3>Sleep and wakeup - usage</h3>
 
 Let's consider implementing a producer/consumer queue
-(like a pipe) that can be used to hold a single non-null char pointer:
+(like a pipe) that can be used to hold a single non-null pointer:
 
 <pre>
 struct pcq {