diff --git a/kernel/proc.c b/kernel/proc.c index 87a81fb..d88e453 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -16,11 +16,14 @@ int nextpid = 1; struct spinlock pid_lock; extern void forkret(void); -static void wakeup1(struct proc *chan); static void freeproc(struct proc *p); extern char trampoline[]; // trampoline.S +// protects parent/child relationships. +// must be held when using p->parent. +// must be acquired before any p->lock. +struct spinlock proc_tree_lock; // Allocate a page for each process's kernel stack. // Map it high in memory, followed by an invalid @@ -45,6 +48,7 @@ procinit(void) struct proc *p; initlock(&pid_lock, "nextpid"); + initlock(&proc_tree_lock, "proc_tree"); for(p = proc; p < &proc[NPROC]; p++) { initlock(&p->lock, "proc"); p->kstack = KSTACK((int) (p - proc)); @@ -113,6 +117,7 @@ allocproc(void) found: p->pid = allocpid(); + p->state = USED; // Allocate a trapframe page. if((p->trapframe = (struct trapframe *)kalloc()) == 0){ @@ -283,8 +288,6 @@ fork(void) } np->sz = p->sz; - np->parent = p; - // copy saved user registers. *(np->trapframe) = *(p->trapframe); @@ -301,35 +304,30 @@ fork(void) pid = np->pid; - np->state = RUNNABLE; + release(&np->lock); + acquire(&proc_tree_lock); + np->parent = p; + release(&proc_tree_lock); + + acquire(&np->lock); + np->state = RUNNABLE; release(&np->lock); return pid; } // Pass p's abandoned children to init. -// Caller must hold p->lock. +// Caller must hold proc_tree_lock. void reparent(struct proc *p) { struct proc *pp; for(pp = proc; pp < &proc[NPROC]; pp++){ - // this code uses pp->parent without holding pp->lock. - // acquiring the lock first could cause a deadlock - // if pp or a child of pp were also in exit() - // and about to try to lock p. if(pp->parent == p){ - // pp->parent can't change between the check and the acquire() - // because only the parent changes it, and we're the parent. - acquire(&pp->lock); pp->parent = initproc; - // we should wake up init here, but that would require - // initproc->lock, which would be a deadlock, since we hold - // the lock on one of init's children (pp). this is why - // exit() always wakes init (before acquiring any locks). - release(&pp->lock); + wakeup(initproc); } } } @@ -359,41 +357,20 @@ exit(int status) end_op(); p->cwd = 0; - // we might re-parent a child to init. we can't be precise about - // waking up init, since we can't acquire its lock once we've - // acquired any other proc lock. so wake up init whether that's - // necessary or not. init may miss this wakeup, but that seems - // harmless. - acquire(&initproc->lock); - wakeup1(initproc); - release(&initproc->lock); - - // grab a copy of p->parent, to ensure that we unlock the same - // parent we locked. in case our parent gives us away to init while - // we're waiting for the parent lock. we may then race with an - // exiting parent, but the result will be a harmless spurious wakeup - // to a dead or wrong process; proc structs are never re-allocated - // as anything else. - acquire(&p->lock); - struct proc *original_parent = p->parent; - release(&p->lock); + acquire(&proc_tree_lock); - // we need the parent's lock in order to wake it up from wait(). - // the parent-then-child rule says we have to lock it first. - acquire(&original_parent->lock); - acquire(&p->lock); // Give any children to init. reparent(p); // Parent might be sleeping in wait(). - wakeup1(original_parent); + wakeup(p->parent); p->xstate = status; p->state = ZOMBIE; - release(&original_parent->lock); + release(&proc_tree_lock); // Jump into the scheduler, never to return. sched(); @@ -409,20 +386,13 @@ wait(uint64 addr) int havekids, pid; struct proc *p = myproc(); - // hold p->lock for the whole time to avoid lost - // wakeups from a child's exit(). - acquire(&p->lock); + acquire(&proc_tree_lock); for(;;){ // Scan through table looking for exited children. havekids = 0; for(np = proc; np < &proc[NPROC]; np++){ - // 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 between the check and the acquire() - // because only the parent changes it, and we're the parent. acquire(&np->lock); havekids = 1; if(np->state == ZOMBIE){ @@ -436,7 +406,7 @@ wait(uint64 addr) } freeproc(np); release(&np->lock); - release(&p->lock); + release(&proc_tree_lock); return pid; } release(&np->lock); @@ -445,12 +415,12 @@ wait(uint64 addr) // No point waiting if we don't have any children. if(!havekids || p->killed){ - release(&p->lock); + release(&proc_tree_lock); return -1; } // Wait for a child to exit. - sleep(p, &p->lock); //DOC: wait-sleep + sleep(p, &proc_tree_lock); //DOC: wait-sleep } } @@ -563,10 +533,9 @@ sleep(void *chan, struct spinlock *lk) // guaranteed that we won't miss any wakeup // (wakeup locks p->lock), // so it's okay to release lk. - if(lk != &p->lock){ //DOC: sleeplock0 - acquire(&p->lock); //DOC: sleeplock1 - release(lk); - } + + acquire(&p->lock); //DOC: sleeplock1 + release(lk); // Go to sleep. p->chan = chan; @@ -578,10 +547,8 @@ sleep(void *chan, struct spinlock *lk) p->chan = 0; // Reacquire original lock. - if(lk != &p->lock){ - release(&p->lock); - acquire(lk); - } + release(&p->lock); + acquire(lk); } // Wake up all processes sleeping on chan. @@ -592,23 +559,13 @@ wakeup(void *chan) struct proc *p; for(p = proc; p < &proc[NPROC]; p++) { - acquire(&p->lock); - if(p->state == SLEEPING && p->chan == chan) { - p->state = RUNNABLE; + if(p != myproc()){ + acquire(&p->lock); + if(p->state == SLEEPING && p->chan == chan) { + p->state = RUNNABLE; + } + release(&p->lock); } - release(&p->lock); - } -} - -// Wake up p if it is sleeping in wait(); used by exit(). -// Caller must hold p->lock. -static void -wakeup1(struct proc *p) -{ - if(!holding(&p->lock)) - panic("wakeup1"); - if(p->chan == p && p->state == SLEEPING) { - p->state = RUNNABLE; } } diff --git a/kernel/proc.h b/kernel/proc.h index 9c16ea7..e86bde1 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -80,7 +80,7 @@ struct trapframe { /* 280 */ uint64 t6; }; -enum procstate { UNUSED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE }; +enum procstate { UNUSED, USED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE }; // Per-process state struct proc {