From 4087a6e7fc773ba4eb217dfc196dfe1eee84b25d Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 10 Aug 2022 20:35:42 -0400 Subject: [PATCH 01/14] 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 02/14] 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 03/14] 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 04/14] 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)) From 7a6d57235c41df877b3fb2322cf770e65e864058 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Fri, 12 Aug 2022 14:59:30 -0400 Subject: [PATCH 05/14] Costmestic change (thanks Harry Porter) --- kernel/syscall.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/kernel/syscall.c b/kernel/syscall.c index 95b9f70..dd7a33e 100644 --- a/kernel/syscall.c +++ b/kernel/syscall.c @@ -12,7 +12,7 @@ int fetchaddr(uint64 addr, uint64 *ip) { struct proc *p = myproc(); - if(addr >= p->sz || addr+sizeof(uint64) > p->sz) + if(addr >= p->sz || addr+sizeof(uint64) > p->sz) // both tests needed, in case of overflow return -1; if(copyin(p->pagetable, (char *)ip, addr, sizeof(*ip)) != 0) return -1; @@ -25,9 +25,8 @@ int fetchstr(uint64 addr, char *buf, int max) { struct proc *p = myproc(); - int err = copyinstr(p->pagetable, buf, addr, max); - if(err < 0) - return err; + if(copyinstr(p->pagetable, buf, addr, max) < 0) + return -1; return strlen(buf); } From 1d4c437ea17e3254fe6138fd8dd181de413a43bf Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 17 Aug 2022 20:37:22 -0400 Subject: [PATCH 06/14] Use uint64 (thanks carlclone and Harry Porter) --- kernel/proc.c | 2 +- kernel/sysproc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index 580b3b6..3bb2618 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -259,7 +259,7 @@ userinit(void) int growproc(int n) { - uint sz; + uint64 sz; struct proc *p = myproc(); sz = p->sz; diff --git a/kernel/sysproc.c b/kernel/sysproc.c index 8ca45ba..c74def2 100644 --- a/kernel/sysproc.c +++ b/kernel/sysproc.c @@ -40,7 +40,7 @@ sys_wait(void) uint64 sys_sbrk(void) { - int addr; + uint64 addr; int n; if(argint(0, &n) < 0) From f2ee8690addf0aa52b5de2061fe6d659574406f9 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Wed, 17 Aug 2022 20:38:11 -0400 Subject: [PATCH 07/14] x --- user/user.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/user.h b/user/user.h index 8ac6395..4d398d5 100644 --- a/user/user.h +++ b/user/user.h @@ -9,7 +9,7 @@ int write(int, const void*, int); int read(int, void*, int); int close(int); int kill(int); -int exec(char*, char**); +int exec(const char*, char**); int open(const char*, int); int mknod(const char*, short, short); int unlink(const char*); From 63ef3b8c9fd15d5ea5775813cda94a3c64cff0d3 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Mon, 22 Aug 2022 13:49:15 -0400 Subject: [PATCH 08/14] slightly better comments --- kernel/trampoline.S | 31 +++++++++++++++---------------- kernel/trap.c | 6 +++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/kernel/trampoline.S b/kernel/trampoline.S index 2ce4886..0aaa413 100644 --- a/kernel/trampoline.S +++ b/kernel/trampoline.S @@ -1,18 +1,19 @@ - # - # code to switch between user and kernel space. # - # this code is mapped at the same virtual address - # (TRAMPOLINE) in user and kernel space so that - # it continues to work when it switches page tables. - # - # kernel.ld causes this to be aligned - # to a page boundary. + # low-level code to handle traps from user space into + # the kernel, and returns from kernel to user. + # + # the kernel maps the page holding this code + # at the same virtual address (TRAMPOLINE) + # in user and kernel space so that it continues + # to work when it switches page tables. + # kernel.ld causes this code to start at + # a page boundary. # #include "riscv.h" #include "memlayout.h" - .section trampsec +.section trampsec .globl trampoline trampoline: .align 4 @@ -31,7 +32,7 @@ uservec: # each process has a separate p->trapframe memory area, # but it's mapped to the same virtual address - # (TRAPFRAME) in every process. + # (TRAPFRAME) in every process's user page table. li a0, TRAPFRAME # save the user registers in TRAPFRAME @@ -70,29 +71,27 @@ uservec: csrr t0, sscratch sd t0, 112(a0) - # restore kernel stack pointer from p->trapframe->kernel_sp + # initialize kernel stack pointer, from p->trapframe->kernel_sp ld sp, 8(a0) # make tp hold the current hartid, from p->trapframe->kernel_hartid ld tp, 32(a0) - # load the address of usertrap(), p->trapframe->kernel_trap + # load the address of usertrap(), from p->trapframe->kernel_trap ld t0, 16(a0) - # restore kernel page table from p->trapframe->kernel_satp + # load the kernel page table, from p->trapframe->kernel_satp ld t1, 0(a0) csrw satp, t1 sfence.vma zero, zero - # a0 is no longer valid, since the kernel page - # table does not specially map p->tf. - # jump to usertrap(), which does not return jr t0 .globl userret userret: # userret(pagetable) + # called by usertrapret() in trap.c to # switch from kernel to user. # a0: user page table, for satp. diff --git a/kernel/trap.c b/kernel/trap.c index 75fb3ec..524da44 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -60,8 +60,8 @@ usertrap(void) // but we want to return to the next instruction. p->trapframe->epc += 4; - // an interrupt will change sstatus &c registers, - // so don't enable until done with those registers. + // an interrupt will change sepc, scause, and sstatus, + // so enable only now that we're done with those registers. intr_on(); syscall(); @@ -101,7 +101,7 @@ usertrapret(void) w_stvec(trampoline_uservec); // set up trapframe values that uservec will need when - // the process next re-enters the kernel. + // the process next traps into the kernel. p->trapframe->kernel_satp = r_satp(); // kernel page table p->trapframe->kernel_sp = p->kstack + PGSIZE; // process's kernel stack p->trapframe->kernel_trap = (uint64)usertrap; From 7086197c27f7c00544ca006561336d8d5791a482 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 22 Aug 2022 19:36:11 -0400 Subject: [PATCH 09/14] Simplify uartputc slightly (thanks Harry Porter) --- kernel/uart.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/kernel/uart.c b/kernel/uart.c index af571b1..e3b3b8a 100644 --- a/kernel/uart.c +++ b/kernel/uart.c @@ -92,22 +92,18 @@ uartputc(int c) for(;;) ; } - - while(1){ - if(uart_tx_w == uart_tx_r + UART_TX_BUF_SIZE){ - // buffer is full. - // wait for uartstart() to open up space in the buffer. - sleep(&uart_tx_r, &uart_tx_lock); - } else { - uart_tx_buf[uart_tx_w % UART_TX_BUF_SIZE] = c; - uart_tx_w += 1; - uartstart(); - release(&uart_tx_lock); - return; - } + while(uart_tx_w == uart_tx_r + UART_TX_BUF_SIZE){ + // buffer is full. + // wait for uartstart() to open up space in the buffer. + sleep(&uart_tx_r, &uart_tx_lock); } + uart_tx_buf[uart_tx_w % UART_TX_BUF_SIZE] = c; + uart_tx_w += 1; + uartstart(); + release(&uart_tx_lock); } + // alternate version of uartputc() that doesn't // use interrupts, for use by kernel printf() and // to echo characters. it spins waiting for the uart's From 2a391ebc8ba1fd0e6f0899277218d531fd5c7396 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 22 Aug 2022 19:53:09 -0400 Subject: [PATCH 10/14] Make argint() and argaddr() of type void (thanks Harry Porter) --- kernel/defs.h | 4 ++-- kernel/syscall.c | 9 +++------ kernel/sysfile.c | 29 +++++++++++++++++------------ kernel/sysproc.c | 15 +++++---------- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/kernel/defs.h b/kernel/defs.h index 62b9292..48641f5 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -132,9 +132,9 @@ int strncmp(const char*, const char*, uint); char* strncpy(char*, const char*, int); // syscall.c -int argint(int, int*); +void argint(int, int*); int argstr(int, char*, int); -int argaddr(int, uint64 *); +void argaddr(int, uint64 *); int fetchstr(uint64, char*, int); int fetchaddr(uint64, uint64*); void syscall(); diff --git a/kernel/syscall.c b/kernel/syscall.c index dd7a33e..ee94696 100644 --- a/kernel/syscall.c +++ b/kernel/syscall.c @@ -53,21 +53,19 @@ argraw(int n) } // Fetch the nth 32-bit system call argument. -int +void argint(int n, int *ip) { *ip = argraw(n); - return 0; } // Retrieve an argument as a pointer. // Doesn't check for legality, since // copyin/copyout will do that. -int +void argaddr(int n, uint64 *ip) { *ip = argraw(n); - return 0; } // Fetch the nth word-sized system call argument as a null-terminated string. @@ -77,8 +75,7 @@ int argstr(int n, char *buf, int max) { uint64 addr; - if(argaddr(n, &addr) < 0) - return -1; + argaddr(n, &addr); return fetchstr(addr, buf, max); } diff --git a/kernel/sysfile.c b/kernel/sysfile.c index 5dc453b..970a72a 100644 --- a/kernel/sysfile.c +++ b/kernel/sysfile.c @@ -24,8 +24,7 @@ argfd(int n, int *pfd, struct file **pf) int fd; struct file *f; - if(argint(n, &fd) < 0) - return -1; + argint(n, &fd); if(fd < 0 || fd >= NOFILE || (f=myproc()->ofile[fd]) == 0) return -1; if(pfd) @@ -73,7 +72,9 @@ sys_read(void) int n; uint64 p; - if(argfd(0, 0, &f) < 0 || argint(2, &n) < 0 || argaddr(1, &p) < 0) + argaddr(1, &p); + argint(2, &n); + if(argfd(0, 0, &f) < 0) return -1; return fileread(f, p, n); } @@ -84,8 +85,10 @@ sys_write(void) struct file *f; int n; uint64 p; - - if(argfd(0, 0, &f) < 0 || argint(2, &n) < 0 || argaddr(1, &p) < 0) + + argaddr(1, &p); + argint(2, &n); + if(argfd(0, 0, &f) < 0) return -1; return filewrite(f, p, n); @@ -110,7 +113,8 @@ sys_fstat(void) struct file *f; uint64 st; // user pointer to struct stat - if(argfd(0, 0, &f) < 0 || argaddr(1, &st) < 0) + argaddr(1, &st); + if(argfd(0, 0, &f) < 0) return -1; return filestat(f, st); } @@ -292,7 +296,8 @@ sys_open(void) struct inode *ip; int n; - if((n = argstr(0, path, MAXPATH)) < 0 || argint(1, &omode) < 0) + argint(1, &omode); + if((n = argstr(0, path, MAXPATH)) < 0) return -1; begin_op(); @@ -375,9 +380,9 @@ sys_mknod(void) int major, minor; begin_op(); + argint(1, &major); + argint(2, &minor); if((argstr(0, path, MAXPATH)) < 0 || - argint(1, &major) < 0 || - argint(2, &minor) < 0 || (ip = create(path, T_DEVICE, major, minor)) == 0){ end_op(); return -1; @@ -419,7 +424,8 @@ sys_exec(void) int i; uint64 uargv, uarg; - if(argstr(0, path, MAXPATH) < 0 || argaddr(1, &uargv) < 0){ + argaddr(1, &uargv); + if(argstr(0, path, MAXPATH) < 0) { return -1; } memset(argv, 0, sizeof(argv)); @@ -462,8 +468,7 @@ sys_pipe(void) int fd0, fd1; struct proc *p = myproc(); - if(argaddr(0, &fdarray) < 0) - return -1; + argaddr(0, &fdarray); if(pipealloc(&rf, &wf) < 0) return -1; fd0 = -1; diff --git a/kernel/sysproc.c b/kernel/sysproc.c index c74def2..ecaa8cc 100644 --- a/kernel/sysproc.c +++ b/kernel/sysproc.c @@ -10,8 +10,7 @@ uint64 sys_exit(void) { int n; - if(argint(0, &n) < 0) - return -1; + argint(0, &n); exit(n); return 0; // not reached } @@ -32,8 +31,7 @@ uint64 sys_wait(void) { uint64 p; - if(argaddr(0, &p) < 0) - return -1; + argaddr(0, &p); return wait(p); } @@ -43,8 +41,7 @@ sys_sbrk(void) uint64 addr; int n; - if(argint(0, &n) < 0) - return -1; + argint(0, &n); addr = myproc()->sz; if(growproc(n) < 0) return -1; @@ -57,8 +54,7 @@ sys_sleep(void) int n; uint ticks0; - if(argint(0, &n) < 0) - return -1; + argint(0, &n); acquire(&tickslock); ticks0 = ticks; while(ticks - ticks0 < n){ @@ -77,8 +73,7 @@ sys_kill(void) { int pid; - if(argint(0, &pid) < 0) - return -1; + argint(0, &pid); return kill(pid); } From 2f0b4d698bc48f4efb4b9fdca20f415ec765bf41 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 22 Aug 2022 19:58:33 -0400 Subject: [PATCH 11/14] Use pp instead of np to be more consistent --- kernel/proc.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index 3bb2618..1ed3ee5 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -390,7 +390,7 @@ exit(int status) int wait(uint64 addr) { - struct proc *np; + struct proc *pp; int havekids, pid; struct proc *p = myproc(); @@ -399,27 +399,27 @@ wait(uint64 addr) for(;;){ // Scan through table looking for exited children. havekids = 0; - for(np = proc; np < &proc[NPROC]; np++){ - if(np->parent == p){ + for(pp = proc; pp < &proc[NPROC]; pp++){ + if(pp->parent == p){ // make sure the child isn't still in exit() or swtch(). - acquire(&np->lock); + acquire(&pp->lock); havekids = 1; - if(np->state == ZOMBIE){ + if(pp->state == ZOMBIE){ // Found one. - pid = np->pid; - if(addr != 0 && copyout(p->pagetable, addr, (char *)&np->xstate, - sizeof(np->xstate)) < 0) { - release(&np->lock); + pid = pp->pid; + if(addr != 0 && copyout(p->pagetable, addr, (char *)&pp->xstate, + sizeof(pp->xstate)) < 0) { + release(&pp->lock); release(&wait_lock); return -1; } - freeproc(np); - release(&np->lock); + freeproc(pp); + release(&pp->lock); release(&wait_lock); return pid; } - release(&np->lock); + release(&pp->lock); } } From bc48c2be47ed4994d23327980737fef441fbbcc8 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 22 Aug 2022 20:44:02 -0400 Subject: [PATCH 12/14] Add ref to this nice page about Chapter 9's regexp matcher --- user/grep.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/user/grep.c b/user/grep.c index 19882b9..2315a0c 100644 --- a/user/grep.c +++ b/user/grep.c @@ -62,7 +62,8 @@ main(int argc, char *argv[]) } // Regexp matcher from Kernighan & Pike, -// The Practice of Programming, Chapter 9. +// The Practice of Programming, Chapter 9, or +// https://www.cs.princeton.edu/courses/archive/spr09/cos333/beautiful.html int matchhere(char*, char*); int matchstar(int, char*, char*); From dc405cdb7b4e4d4bb3fc50b3e7f44e8795c0218e Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Tue, 23 Aug 2022 08:23:12 -0400 Subject: [PATCH 13/14] don't panic if out of disk space when extending a directory. --- kernel/fs.c | 3 ++- kernel/sysfile.c | 9 +++++++-- user/usertests.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/kernel/fs.c b/kernel/fs.c index 247a86f..b220491 100644 --- a/kernel/fs.c +++ b/kernel/fs.c @@ -573,6 +573,7 @@ dirlookup(struct inode *dp, char *name, uint *poff) } // Write a new directory entry (name, inum) into the directory dp. +// Returns 0 on success, -1 on failure (e.g. out of disk blocks). int dirlink(struct inode *dp, char *name, uint inum) { @@ -597,7 +598,7 @@ dirlink(struct inode *dp, char *name, uint inum) strncpy(de.name, name, DIRSIZ); de.inum = inum; if(writei(dp, 0, (uint64)&de, off, sizeof(de)) != sizeof(de)) - panic("dirlink"); + return -1; return 0; } diff --git a/kernel/sysfile.c b/kernel/sysfile.c index 970a72a..4c0470e 100644 --- a/kernel/sysfile.c +++ b/kernel/sysfile.c @@ -279,8 +279,13 @@ create(char *path, short type, short major, short minor) panic("create dots"); } - if(dirlink(dp, name, ip->inum) < 0) - panic("create: dirlink"); + if(dirlink(dp, name, ip->inum) < 0){ + // oops. we don't need ip after all. + ip->nlink = 0; + iupdate(ip); + iunlockput(ip); + ip = 0; + } iunlockput(dp); diff --git a/user/usertests.c b/user/usertests.c index 4ab34da..968034a 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -2738,6 +2738,36 @@ diskfull(char *s) close(fd); } + // now that there are no free blocks, test that dirlink() + // merely fails (doesn't panic) if it can't extend + // directory content. + int nzz = 128; + for(int i = 0; i < nzz; i++){ + char name[32]; + name[0] = 'z'; + name[1] = 'z'; + name[2] = '0' + (i / 32); + name[3] = '0' + (i % 32); + name[4] = '\0'; + unlink(name); + int fd = open(name, O_CREATE|O_RDWR|O_TRUNC); + if(fd < 0){ + printf("%s: could not create file %s\n", s, name); + break; + } + close(fd); + } + + for(int i = 0; i < nzz; i++){ + char name[32]; + name[0] = 'z'; + name[1] = 'z'; + name[2] = '0' + (i / 32); + name[3] = '0' + (i % 32); + name[4] = '\0'; + unlink(name); + } + for(int i = 0; i < fi; i++){ char name[32]; name[0] = 'b'; From 8621be8f3d105cd73ffbc681f9810d04b083b0ae Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Tue, 23 Aug 2022 08:52:15 -0400 Subject: [PATCH 14/14] tolerate out of disk when creating . and .. in mkdir() --- kernel/sysfile.c | 25 ++++++++++++++++--------- user/usertests.c | 5 +++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/kernel/sysfile.c b/kernel/sysfile.c index 4c0470e..d8a6fca 100644 --- a/kernel/sysfile.c +++ b/kernel/sysfile.c @@ -272,24 +272,31 @@ create(char *path, short type, short major, short minor) iupdate(ip); if(type == T_DIR){ // Create . and .. entries. - dp->nlink++; // for ".." - iupdate(dp); // No ip->nlink++ for ".": avoid cyclic ref count. if(dirlink(ip, ".", ip->inum) < 0 || dirlink(ip, "..", dp->inum) < 0) - panic("create dots"); + goto fail; } - if(dirlink(dp, name, ip->inum) < 0){ - // oops. we don't need ip after all. - ip->nlink = 0; - iupdate(ip); - iunlockput(ip); - ip = 0; + if(dirlink(dp, name, ip->inum) < 0) + goto fail; + + if(type == T_DIR){ + // now that success is guaranteed: + dp->nlink++; // for ".." + iupdate(dp); } iunlockput(dp); return ip; + + fail: + // something went wrong. de-allocate ip. + ip->nlink = 0; + iupdate(ip); + iunlockput(ip); + iunlockput(dp); + return 0; } uint64 diff --git a/user/usertests.c b/user/usertests.c index 968034a..60d1762 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -2712,6 +2712,8 @@ diskfull(char *s) { int fi; int done = 0; + + unlink("diskfulldir"); for(fi = 0; done == 0; fi++){ char name[32]; @@ -2758,6 +2760,9 @@ diskfull(char *s) close(fd); } + mkdir("diskfulldir"); + unlink("diskfulldir"); + for(int i = 0; i < nzz; i++){ char name[32]; name[0] = 'z';