From 4087a6e7fc773ba4eb217dfc196dfe1eee84b25d Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 10 Aug 2022 20:35:42 -0400 Subject: [PATCH 1/4] Read and write p->killed using atomics --- kernel/console.c | 2 +- kernel/pipe.c | 4 ++-- kernel/proc.c | 4 ++-- kernel/sysproc.c | 2 +- kernel/trap.c | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/kernel/console.c b/kernel/console.c index 23a2d35..b8fa1de 100644 --- a/kernel/console.c +++ b/kernel/console.c @@ -89,7 +89,7 @@ consoleread(int user_dst, uint64 dst, int n) // wait until interrupt handler has put some // input into cons.buffer. while(cons.r == cons.w){ - if(myproc()->killed){ + if(__sync_add_and_fetch(&(myproc()->killed), 0)){ release(&cons.lock); return -1; } diff --git a/kernel/pipe.c b/kernel/pipe.c index b6eefb9..e438d7e 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -81,7 +81,7 @@ pipewrite(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); while(i < n){ - if(pi->readopen == 0 || pr->killed){ + if(pi->readopen == 0 || __sync_add_and_fetch(&pr->killed,0)){ release(&pi->lock); return -1; } @@ -111,7 +111,7 @@ piperead(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); while(pi->nread == pi->nwrite && pi->writeopen){ //DOC: pipe-empty - if(pr->killed){ + if(__sync_add_and_fetch(&pr->killed,0)){ release(&pi->lock); return -1; } diff --git a/kernel/proc.c b/kernel/proc.c index 2d0ffa1..221f0f8 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -422,7 +422,7 @@ wait(uint64 addr) } // No point waiting if we don't have any children. - if(!havekids || p->killed){ + if(!havekids || __sync_add_and_fetch(&p->killed, 0)){ release(&wait_lock); return -1; } @@ -588,7 +588,7 @@ kill(int pid) for(p = proc; p < &proc[NPROC]; p++){ acquire(&p->lock); if(p->pid == pid){ - p->killed = 1; + __sync_bool_compare_and_swap(&p->killed, 0, 1); if(p->state == SLEEPING){ // Wake process from sleep(). p->state = RUNNABLE; diff --git a/kernel/sysproc.c b/kernel/sysproc.c index e8bcda9..61b715b 100644 --- a/kernel/sysproc.c +++ b/kernel/sysproc.c @@ -63,7 +63,7 @@ sys_sleep(void) acquire(&tickslock); ticks0 = ticks; while(ticks - ticks0 < n){ - if(myproc()->killed){ + if(__sync_add_and_fetch(&(myproc()->killed), 0)){ release(&tickslock); return -1; } diff --git a/kernel/trap.c b/kernel/trap.c index 75fb3ec..b879f01 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -53,7 +53,7 @@ usertrap(void) if(r_scause() == 8){ // system call - if(p->killed) + if(__sync_add_and_fetch(&p->killed, 0)) exit(-1); // sepc points to the ecall instruction, @@ -70,10 +70,10 @@ usertrap(void) } else { printf("usertrap(): unexpected scause %p pid=%d\n", r_scause(), p->pid); printf(" sepc=%p stval=%p\n", r_sepc(), r_stval()); - p->killed = 1; + __sync_bool_compare_and_swap(&p->killed, 0, 1); } - if(p->killed) + if(__sync_add_and_fetch(&p->killed, 0)) exit(-1); // give up the CPU if this is a timer interrupt. From 975f3b31d3fac2c271df3107263df6ae454a98be Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Thu, 11 Aug 2022 07:23:17 -0400 Subject: [PATCH 2/4] Clean up using killed() --- kernel/console.c | 2 +- kernel/defs.h | 1 + kernel/pipe.c | 4 ++-- kernel/proc.c | 8 +++++++- kernel/sysproc.c | 2 +- kernel/trap.c | 4 ++-- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/kernel/console.c b/kernel/console.c index b8fa1de..d6eb209 100644 --- a/kernel/console.c +++ b/kernel/console.c @@ -89,7 +89,7 @@ consoleread(int user_dst, uint64 dst, int n) // wait until interrupt handler has put some // input into cons.buffer. while(cons.r == cons.w){ - if(__sync_add_and_fetch(&(myproc()->killed), 0)){ + if(killed(myproc())){ release(&cons.lock); return -1; } diff --git a/kernel/defs.h b/kernel/defs.h index 62b9292..7181d4d 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -90,6 +90,7 @@ void proc_mapstacks(pagetable_t); pagetable_t proc_pagetable(struct proc *); void proc_freepagetable(pagetable_t, uint64); int kill(int); +int killed(struct proc*); struct cpu* mycpu(void); struct cpu* getmycpu(void); struct proc* myproc(); diff --git a/kernel/pipe.c b/kernel/pipe.c index e438d7e..f6b501a 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -81,7 +81,7 @@ pipewrite(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); while(i < n){ - if(pi->readopen == 0 || __sync_add_and_fetch(&pr->killed,0)){ + if(pi->readopen == 0 || killed(pr)){ release(&pi->lock); return -1; } @@ -111,7 +111,7 @@ piperead(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); while(pi->nread == pi->nwrite && pi->writeopen){ //DOC: pipe-empty - if(__sync_add_and_fetch(&pr->killed,0)){ + if(killed(pr)){ release(&pi->lock); return -1; } diff --git a/kernel/proc.c b/kernel/proc.c index 221f0f8..24680b6 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -422,7 +422,7 @@ wait(uint64 addr) } // No point waiting if we don't have any children. - if(!havekids || __sync_add_and_fetch(&p->killed, 0)){ + if(!havekids || killed(p)){ release(&wait_lock); return -1; } @@ -601,6 +601,12 @@ kill(int pid) return -1; } +int +killed(struct proc *p) +{ + return __sync_add_and_fetch(&p->killed, 0); +} + // Copy to either a user address, or kernel address, // depending on usr_dst. // Returns 0 on success, -1 on error. diff --git a/kernel/sysproc.c b/kernel/sysproc.c index 61b715b..99a36a7 100644 --- a/kernel/sysproc.c +++ b/kernel/sysproc.c @@ -63,7 +63,7 @@ sys_sleep(void) acquire(&tickslock); ticks0 = ticks; while(ticks - ticks0 < n){ - if(__sync_add_and_fetch(&(myproc()->killed), 0)){ + if(killed(myproc())){ release(&tickslock); return -1; } diff --git a/kernel/trap.c b/kernel/trap.c index b879f01..f895aea 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -53,7 +53,7 @@ usertrap(void) if(r_scause() == 8){ // system call - if(__sync_add_and_fetch(&p->killed, 0)) + if(killed(p)) exit(-1); // sepc points to the ecall instruction, @@ -73,7 +73,7 @@ usertrap(void) __sync_bool_compare_and_swap(&p->killed, 0, 1); } - if(__sync_add_and_fetch(&p->killed, 0)) + if(killed(p)) exit(-1); // give up the CPU if this is a timer interrupt. From 429c7b717edd4c23d3666327986052b9b6eb29eb Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Thu, 11 Aug 2022 08:42:52 -0400 Subject: [PATCH 3/4] Use atomic store_n and load_n --- kernel/proc.c | 4 ++-- kernel/trap.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index 24680b6..b4f8a80 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -588,7 +588,7 @@ kill(int pid) for(p = proc; p < &proc[NPROC]; p++){ acquire(&p->lock); if(p->pid == pid){ - __sync_bool_compare_and_swap(&p->killed, 0, 1); + __atomic_store_n(&p->killed, 1, __ATOMIC_SEQ_CST); if(p->state == SLEEPING){ // Wake process from sleep(). p->state = RUNNABLE; @@ -604,7 +604,7 @@ kill(int pid) int killed(struct proc *p) { - return __sync_add_and_fetch(&p->killed, 0); + return __atomic_load_n(&p->killed, __ATOMIC_SEQ_CST); } // Copy to either a user address, or kernel address, diff --git a/kernel/trap.c b/kernel/trap.c index f895aea..1039911 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -70,7 +70,7 @@ usertrap(void) } else { printf("usertrap(): unexpected scause %p pid=%d\n", r_scause(), p->pid); printf(" sepc=%p stval=%p\n", r_sepc(), r_stval()); - __sync_bool_compare_and_swap(&p->killed, 0, 1); + __atomic_store_n(&p->killed, 1, __ATOMIC_SEQ_CST); } if(killed(p)) From 4f716c8550b406c3e4b3e0c21b986ef99bc06c40 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Thu, 11 Aug 2022 14:22:00 -0400 Subject: [PATCH 4/4] Use p->lock to read p->killed --- kernel/defs.h | 1 + kernel/proc.c | 17 +++++++++++++++-- kernel/trap.c | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/kernel/defs.h b/kernel/defs.h index 7181d4d..e38ec00 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -91,6 +91,7 @@ pagetable_t proc_pagetable(struct proc *); void proc_freepagetable(pagetable_t, uint64); int kill(int); int killed(struct proc*); +void setkilled(struct proc*); struct cpu* mycpu(void); struct cpu* getmycpu(void); struct proc* myproc(); diff --git a/kernel/proc.c b/kernel/proc.c index b4f8a80..5e37cb7 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -588,7 +588,7 @@ kill(int pid) for(p = proc; p < &proc[NPROC]; p++){ acquire(&p->lock); if(p->pid == pid){ - __atomic_store_n(&p->killed, 1, __ATOMIC_SEQ_CST); + p->killed = 1; if(p->state == SLEEPING){ // Wake process from sleep(). p->state = RUNNABLE; @@ -601,10 +601,23 @@ kill(int pid) return -1; } +void +setkilled(struct proc *p) +{ + acquire(&p->lock); + p->killed = 1; + release(&p->lock); +} + int killed(struct proc *p) { - return __atomic_load_n(&p->killed, __ATOMIC_SEQ_CST); + int k; + + acquire(&p->lock); + k = p->killed; + release(&p->lock); + return k; } // Copy to either a user address, or kernel address, diff --git a/kernel/trap.c b/kernel/trap.c index 1039911..44c9cdc 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -70,7 +70,7 @@ usertrap(void) } else { printf("usertrap(): unexpected scause %p pid=%d\n", r_scause(), p->pid); printf(" sepc=%p stval=%p\n", r_sepc(), r_stval()); - __atomic_store_n(&p->killed, 1, __ATOMIC_SEQ_CST); + setkilled(p); } if(killed(p))