From d63ac118e817ecab6fd4f890960f4f73b4dfd5e8 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 8 Sep 2016 14:22:38 -0400 Subject: [PATCH 1/2] this might fix the reported deadlock, though I can't reproduce it. --- proc.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/proc.c b/proc.c index 9e9552936c..eee79af7d2 100644 --- a/proc.c +++ b/proc.c @@ -82,6 +82,12 @@ userinit(void) acquire(&ptable.lock); p = allocproc(); + + // release the lock in case namei() sleeps. + // the lock isn't needed because no other + // thread will look at an EMBRYO proc. + release(&ptable.lock); + initproc = p; if((p->pgdir = setupkvm()) == 0) panic("userinit: out of memory?"); @@ -99,6 +105,12 @@ userinit(void) safestrcpy(p->name, "initcode", sizeof(p->name)); p->cwd = namei("/"); + // this assignment to p->state lets other cores + // run this process. the acquire forces the above + // writes to be visible, and the lock is also needed + // because the assignment might not be atomic. + acquire(&ptable.lock); + p->state = RUNNABLE; release(&ptable.lock); @@ -141,6 +153,8 @@ fork(void) return -1; } + release(&ptable.lock); + // Copy process state from p. if((np->pgdir = copyuvm(proc->pgdir, proc->sz)) == 0){ kfree(np->kstack); @@ -165,6 +179,8 @@ fork(void) pid = np->pid; + acquire(&ptable.lock); + np->state = RUNNABLE; release(&ptable.lock); @@ -227,7 +243,7 @@ wait(void) acquire(&ptable.lock); for(;;){ - // Scan through table looking for zombie children. + // Scan through table looking for exited children. havekids = 0; for(p = ptable.proc; p < &ptable.proc[NPROC]; p++){ if(p->parent != proc) From 34c2efc1d063f4b366c1017c174d117cc96eb990 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 8 Sep 2016 14:45:20 -0400 Subject: [PATCH 2/2] use asm() for lock release, not a C assignment --- spinlock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spinlock.c b/spinlock.c index bf863ef104..7b372ef3f1 100644 --- a/spinlock.c +++ b/spinlock.c @@ -59,8 +59,10 @@ release(struct spinlock *lk) // stores; __sync_synchronize() tells them both to not re-order. __sync_synchronize(); - // Release the lock. - lk->locked = 0; + // Release the lock, equivalent to lk->locked = 0. + // This code can't use a C assignment, since it might + // not be atomic. + asm volatile("movl $0, %0" : "+m" (lk->locked) : ); popcli(); }