From 4e62de64cd3b8b67bdb2c3d8edab1ca353427a84 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 25 Jul 2019 06:30:49 -0400 Subject: [PATCH] fix an exit/exit deadlock -> one more locking protocol violation increase timer rate from 1/second to 10/second --- kernel/proc.c | 24 ++++++++++++++---------- kernel/start.c | 9 +++++---- user/usertests.c | 6 +++--- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index 5136766..48b006f 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -294,17 +294,21 @@ reparent(struct proc *p, struct proc *parent) { int child_of_init = (p->parent == initproc); for(pp = proc; pp < &proc[NPROC]; pp++){ - if (pp != p && pp != parent) { + // 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); - if(pp->parent == p){ - pp->parent = initproc; - if(pp->state == ZOMBIE) { - if(!child_of_init) - acquire(&initproc->lock); - wakeup1(initproc); - if(!child_of_init) - release(&initproc->lock); - } + pp->parent = initproc; + if(pp->state == ZOMBIE) { + if(!child_of_init) + acquire(&initproc->lock); + wakeup1(initproc); + if(!child_of_init) + release(&initproc->lock); } release(&pp->lock); } diff --git a/kernel/start.c b/kernel/start.c index 6dc106d..d1fc9c6 100644 --- a/kernel/start.c +++ b/kernel/start.c @@ -41,15 +41,16 @@ start() // which turns them into software interrupts for // devintr() in trap.c. int id = r_mhartid(); - // ask the CLINT for a timer interrupt 10,000 cycles from now. - *(uint64*)CLINT_MTIMECMP(id) = *(uint64*)CLINT_MTIME + 10000; + // ask the CLINT for a timer interrupt. + int interval = 1000000; // cycles; about 1/10th second in qemu. + *(uint64*)CLINT_MTIMECMP(id) = *(uint64*)CLINT_MTIME + interval; // prepare information in scratch[] for machinevec. // scratch[0..3] : space for machinevec to save registers. // scratch[4] : address of CLINT MTIMECMP register. // scratch[5] : desired interval (in cycles) between timer interrupts. uint64 *scratch = &mscratch0[32 * id]; scratch[4] = CLINT_MTIMECMP(id); - scratch[5] = 10000000; + scratch[5] = interval; w_mscratch((uint64)scratch); // set the machine-mode trap handler. w_mtvec((uint64)machinevec); @@ -61,6 +62,6 @@ start() // keep each CPU's hartid in its tp register, for cpuid(). w_tp(id); - // call main() in supervisor mode. + // switch to supervisor mode and jump to main(). asm volatile("mret"); } diff --git a/user/usertests.c b/user/usertests.c index 3d8786c..f74b88c 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -448,7 +448,7 @@ reparent(void) printf(1, "reparent test\n"); - for(int i = 0; i < 100; i++){ + for(int i = 0; i < 200; i++){ int pid = fork(); if(pid < 0){ printf(1, "fork failed\n"); @@ -571,10 +571,10 @@ forkforkfork(void) exit(); } - sleep(2); + sleep(20); // two seconds close(open("stopforking", O_CREATE|O_RDWR)); wait(); - sleep(1); + sleep(10); // one second printf(1, "forkforkfork ok\n"); }