more doc tweaks

This commit is contained in:
Russ Cox 2009-07-12 18:33:37 -07:00
parent 2c5f7aba38
commit 00e571155c
4 changed files with 52 additions and 54 deletions

26
TRICKS
View file

@ -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. forkret1 in trapasm.S is called with a tf argument.
In order to use it, forkret1 copies the tf pointer into In order to use it, forkret1 copies the tf pointer into
%esp and then jumps to trapret, which pops the %esp and then jumps to trapret, which pops the
@ -45,21 +48,21 @@ always.
There is a (harmless) race in pushcli, which does There is a (harmless) race in pushcli, which does
eflags = read_eflags(); eflags = readeflags();
cli(); cli();
if(cpus[cpu()].ncli++ == 0) if(c->ncli++ == 0)
cpus[cpu()].intena = eflags & FL_IF; c->intena = eflags & FL_IF;
Consider a bottom-level pushcli. Consider a bottom-level pushcli.
If interrupts are disabled already, then the right thing If interrupts are disabled already, then the right thing
happens: read_eflags finds that FL_IF is not set, 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: 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, can get preempted and rescheduled on another cpu,
and then once it starts running, perhaps with and then once it starts running, perhaps with
interrupts disabled (can happen since the scheduler 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 not every time it schedules a process), it will
incorrectly record that interrupts *were* enabled. incorrectly record that interrupts *were* enabled.
This doesn't matter, because if it was safe to be 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. setting np->state to RUNNABLE.
int int
sys_fork(void) fork(void)
{ {
int pid; ...
struct proc *np;
if((np = copyproc(cp)) == 0)
return -1;
pid = np->pid; pid = np->pid;
np->state = RUNNABLE; np->state = RUNNABLE;
return pid; 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 before the return statement. So it's not safe to just do
"return np->pid;". "return np->pid;".
This works because proc.h marks the pid as volatile.

40
pipe.c
View file

@ -9,12 +9,12 @@
#define PIPESIZE 512 #define PIPESIZE 512
struct pipe { 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; struct spinlock lock;
char data[PIPESIZE]; 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 int
@ -30,8 +30,8 @@ pipealloc(struct file **f0, struct file **f1)
goto bad; goto bad;
p->readopen = 1; p->readopen = 1;
p->writeopen = 1; p->writeopen = 1;
p->writep = 0; p->nwrite = 0;
p->readp = 0; p->nread = 0;
initlock(&p->lock, "pipe"); initlock(&p->lock, "pipe");
(*f0)->type = FD_PIPE; (*f0)->type = FD_PIPE;
(*f0)->readable = 1; (*f0)->readable = 1;
@ -60,10 +60,10 @@ pipeclose(struct pipe *p, int writable)
acquire(&p->lock); acquire(&p->lock);
if(writable){ if(writable){
p->writeopen = 0; p->writeopen = 0;
wakeup(&p->readp); wakeup(&p->nread);
} else { } else {
p->readopen = 0; p->readopen = 0;
wakeup(&p->writep); wakeup(&p->nwrite);
} }
if(p->readopen == 0 && p->writeopen == 0) { if(p->readopen == 0 && p->writeopen == 0) {
release(&p->lock); release(&p->lock);
@ -80,19 +80,19 @@ pipewrite(struct pipe *p, char *addr, int n)
acquire(&p->lock); acquire(&p->lock);
for(i = 0; i < n; i++){ 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){ if(p->readopen == 0 || cp->killed){
release(&p->lock); release(&p->lock);
return -1; return -1;
} }
wakeup(&p->readp); wakeup(&p->nread);
sleep(&p->writep, &p->lock); 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); release(&p->lock);
return i; return n;
} }
int int
@ -101,19 +101,19 @@ piperead(struct pipe *p, char *addr, int n)
int i; int i;
acquire(&p->lock); acquire(&p->lock);
while(p->readp == p->writep && p->writeopen){ while(p->nread == p->nwrite && p->writeopen){ //DOC: pipe-empty
if(cp->killed){ if(cp->killed){
release(&p->lock); release(&p->lock);
return -1; return -1;
} }
sleep(&p->readp, &p->lock); sleep(&p->nread, &p->lock); //DOC: piperead-sleep
} }
for(i = 0; i < n; i++){ for(i = 0; i < n; i++){ //DOC: piperead-copy
if(p->readp == p->writep) if(p->nread == p->nwrite)
break; 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); release(&p->lock);
return i; return i;
} }

16
proc.c
View file

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

View file

@ -47,7 +47,7 @@
<h3>Sleep and wakeup - usage</h3> <h3>Sleep and wakeup - usage</h3>
Let's consider implementing a producer/consumer queue 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> <pre>
struct pcq { struct pcq {