Frans' proc_lock.

This commit is contained in:
Robert Morris 2020-11-03 15:02:08 -05:00
parent af570f582c
commit 8d4fbc6e2a
2 changed files with 34 additions and 77 deletions

View file

@ -16,11 +16,14 @@ int nextpid = 1;
struct spinlock pid_lock; struct spinlock pid_lock;
extern void forkret(void); extern void forkret(void);
static void wakeup1(struct proc *chan);
static void freeproc(struct proc *p); static void freeproc(struct proc *p);
extern char trampoline[]; // trampoline.S 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. // Allocate a page for each process's kernel stack.
// Map it high in memory, followed by an invalid // Map it high in memory, followed by an invalid
@ -45,6 +48,7 @@ procinit(void)
struct proc *p; struct proc *p;
initlock(&pid_lock, "nextpid"); initlock(&pid_lock, "nextpid");
initlock(&proc_tree_lock, "proc_tree");
for(p = proc; p < &proc[NPROC]; p++) { for(p = proc; p < &proc[NPROC]; p++) {
initlock(&p->lock, "proc"); initlock(&p->lock, "proc");
p->kstack = KSTACK((int) (p - proc)); p->kstack = KSTACK((int) (p - proc));
@ -113,6 +117,7 @@ allocproc(void)
found: found:
p->pid = allocpid(); p->pid = allocpid();
p->state = USED;
// Allocate a trapframe page. // Allocate a trapframe page.
if((p->trapframe = (struct trapframe *)kalloc()) == 0){ if((p->trapframe = (struct trapframe *)kalloc()) == 0){
@ -283,8 +288,6 @@ fork(void)
} }
np->sz = p->sz; np->sz = p->sz;
np->parent = p;
// copy saved user registers. // copy saved user registers.
*(np->trapframe) = *(p->trapframe); *(np->trapframe) = *(p->trapframe);
@ -301,35 +304,30 @@ fork(void)
pid = np->pid; 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); release(&np->lock);
return pid; return pid;
} }
// Pass p's abandoned children to init. // Pass p's abandoned children to init.
// Caller must hold p->lock. // Caller must hold proc_tree_lock.
void void
reparent(struct proc *p) reparent(struct proc *p)
{ {
struct proc *pp; struct proc *pp;
for(pp = proc; pp < &proc[NPROC]; 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){ 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; pp->parent = initproc;
// we should wake up init here, but that would require wakeup(initproc);
// 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);
} }
} }
} }
@ -359,28 +357,7 @@ exit(int status)
end_op(); end_op();
p->cwd = 0; p->cwd = 0;
// we might re-parent a child to init. we can't be precise about acquire(&proc_tree_lock);
// 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);
// 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); acquire(&p->lock);
@ -388,12 +365,12 @@ exit(int status)
reparent(p); reparent(p);
// Parent might be sleeping in wait(). // Parent might be sleeping in wait().
wakeup1(original_parent); wakeup(p->parent);
p->xstate = status; p->xstate = status;
p->state = ZOMBIE; p->state = ZOMBIE;
release(&original_parent->lock); release(&proc_tree_lock);
// Jump into the scheduler, never to return. // Jump into the scheduler, never to return.
sched(); sched();
@ -409,20 +386,13 @@ wait(uint64 addr)
int havekids, pid; int havekids, pid;
struct proc *p = myproc(); struct proc *p = myproc();
// hold p->lock for the whole time to avoid lost acquire(&proc_tree_lock);
// wakeups from a child's exit().
acquire(&p->lock);
for(;;){ for(;;){
// Scan through table looking for exited children. // Scan through table looking for exited children.
havekids = 0; havekids = 0;
for(np = proc; np < &proc[NPROC]; np++){ 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){ 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); acquire(&np->lock);
havekids = 1; havekids = 1;
if(np->state == ZOMBIE){ if(np->state == ZOMBIE){
@ -436,7 +406,7 @@ wait(uint64 addr)
} }
freeproc(np); freeproc(np);
release(&np->lock); release(&np->lock);
release(&p->lock); release(&proc_tree_lock);
return pid; return pid;
} }
release(&np->lock); release(&np->lock);
@ -445,12 +415,12 @@ wait(uint64 addr)
// No point waiting if we don't have any children. // No point waiting if we don't have any children.
if(!havekids || p->killed){ if(!havekids || p->killed){
release(&p->lock); release(&proc_tree_lock);
return -1; return -1;
} }
// Wait for a child to exit. // 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 // guaranteed that we won't miss any wakeup
// (wakeup locks p->lock), // (wakeup locks p->lock),
// so it's okay to release lk. // so it's okay to release lk.
if(lk != &p->lock){ //DOC: sleeplock0
acquire(&p->lock); //DOC: sleeplock1 acquire(&p->lock); //DOC: sleeplock1
release(lk); release(lk);
}
// Go to sleep. // Go to sleep.
p->chan = chan; p->chan = chan;
@ -578,11 +547,9 @@ sleep(void *chan, struct spinlock *lk)
p->chan = 0; p->chan = 0;
// Reacquire original lock. // Reacquire original lock.
if(lk != &p->lock){
release(&p->lock); release(&p->lock);
acquire(lk); acquire(lk);
} }
}
// Wake up all processes sleeping on chan. // Wake up all processes sleeping on chan.
// Must be called without any p->lock. // Must be called without any p->lock.
@ -592,6 +559,7 @@ wakeup(void *chan)
struct proc *p; struct proc *p;
for(p = proc; p < &proc[NPROC]; p++) { for(p = proc; p < &proc[NPROC]; p++) {
if(p != myproc()){
acquire(&p->lock); acquire(&p->lock);
if(p->state == SLEEPING && p->chan == chan) { if(p->state == SLEEPING && p->chan == chan) {
p->state = RUNNABLE; p->state = RUNNABLE;
@ -599,17 +567,6 @@ wakeup(void *chan)
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;
}
} }
// Kill the process with the given pid. // Kill the process with the given pid.

View file

@ -80,7 +80,7 @@ struct trapframe {
/* 280 */ uint64 t6; /* 280 */ uint64 t6;
}; };
enum procstate { UNUSED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE }; enum procstate { UNUSED, USED, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
// Per-process state // Per-process state
struct proc { struct proc {