diff --git a/TRICKS b/TRICKS
index b5388343dc2a60d3945eb9f11dea9d94a07160a1..03382790667531c418bd3b0efd255a433b310968 100644
--- a/TRICKS
+++ b/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.
diff --git a/pipe.c b/pipe.c
index 7f18e98ba5300ed6252de148362a19ea54db4d1c..3032775f412f8ee885cf45ba2831a34edc082220 100644
--- a/pipe.c
+++ b/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;
}
diff --git a/proc.c b/proc.c
index aa6c4ec45f5086dc0c54e0cc569d6e7fd80f38cb..43335122c299bb180090c90a465d174b601a6dcd 100644
--- a/proc.c
+++ b/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
}
}
diff --git a/web/l-coordination.html b/web/l-coordination.html
index b2f9f0dae57f4792953495e293436247bdda5a84..79b578b1e5f2b87c34c730fb0672e945e56578fb 100644
--- a/web/l-coordination.html
+++ b/web/l-coordination.html
@@ -47,7 +47,7 @@
<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 {