From deaff5d8a689e6aa7b64b38619cf667b963256da Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Tue, 24 Sep 2019 14:41:51 -0400 Subject: [PATCH 01/31] no buf->qnext --- kernel/buf.h | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/buf.h b/kernel/buf.h index 4a3a39d..4616e9e 100644 --- a/kernel/buf.h +++ b/kernel/buf.h @@ -7,7 +7,6 @@ struct buf { uint refcnt; struct buf *prev; // LRU cache list struct buf *next; - struct buf *qnext; // disk queue uchar data[BSIZE]; }; From d4416744777740f74a19294d332697639714d8d4 Mon Sep 17 00:00:00 2001 From: Anish Athalye Date: Mon, 30 Sep 2019 20:15:19 -0400 Subject: [PATCH 02/31] Make QEMU memory size match PHYSTOP --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a398c5a..ba7a6fb 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ CPUS := 3 endif QEMUEXTRA = -drive file=fs1.img,if=none,format=raw,id=x1 -device virtio-blk-device,drive=x1,bus=virtio-mmio-bus.1 -QEMUOPTS = -machine virt -bios none -kernel $K/kernel -m 3G -smp $(CPUS) -nographic +QEMUOPTS = -machine virt -bios none -kernel $K/kernel -m 128M -smp $(CPUS) -nographic QEMUOPTS += -drive file=fs.img,if=none,format=raw,id=x0 -device virtio-blk-device,drive=x0,bus=virtio-mmio-bus.0 qemu: $K/kernel fs.img From 78f863f8aead6346dfdfc62e91af25c9383e25a7 Mon Sep 17 00:00:00 2001 From: Anish Athalye Date: Mon, 30 Sep 2019 20:38:17 -0400 Subject: [PATCH 03/31] Add editorconfig --- .editorconfig | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..cd9dfc3 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,19 @@ +; https://editorconfig.org + +root = true + +[*] +end_of_line = lf +insert_final_newline = true +indent_style = space +indent_size = 4 + +[*.{c,h}] +indent_size = 2 + +[*.S] +indent_size = 8 + +[Makefile] +indent_style = tab +indent_size = 8 From 56583b1402a7f8fad0f8c3c296e26f12b1114c95 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 3 Oct 2019 15:02:19 -0400 Subject: [PATCH 04/31] updated alarmtest --- kernel/defs.h | 1 - kernel/exec.c | 1 + kernel/plic.c | 14 -------------- kernel/proc.h | 2 +- kernel/spinlock.c | 9 ++++++--- user/alarmtest.c | 30 ++++++++++++++++++++++++------ 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/kernel/defs.h b/kernel/defs.h index f893d28..9c5f643 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -173,7 +173,6 @@ int copyinstr(pagetable_t, char *, uint64, uint64); // plic.c void plicinit(void); void plicinithart(void); -uint64 plic_pending(void); int plic_claim(void); void plic_complete(int); diff --git a/kernel/exec.c b/kernel/exec.c index 74ef654..6a51499 100644 --- a/kernel/exec.c +++ b/kernel/exec.c @@ -112,6 +112,7 @@ exec(char *path, char **argv) p->tf->epc = elf.entry; // initial program counter = main p->tf->sp = sp; // initial stack pointer proc_freepagetable(oldpagetable, oldsz); + return argc; // this ends up in a0, the first argument to main(argc, argv) bad: diff --git a/kernel/plic.c b/kernel/plic.c index b569492..eed8316 100644 --- a/kernel/plic.c +++ b/kernel/plic.c @@ -28,20 +28,6 @@ plicinithart(void) *(uint32*)PLIC_SPRIORITY(hart) = 0; } -// return a bitmap of which IRQs are waiting -// to be served. -uint64 -plic_pending(void) -{ - uint64 mask; - - //mask = *(uint32*)(PLIC + 0x1000); - //mask |= (uint64)*(uint32*)(PLIC + 0x1004) << 32; - mask = *(uint64*)PLIC_PENDING; - - return mask; -} - // ask the PLIC what interrupt we should serve. int plic_claim(void) diff --git a/kernel/proc.h b/kernel/proc.h index 538b48a..812c769 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -95,7 +95,7 @@ struct proc { int pid; // Process ID // these are private to the process, so p->lock need not be held. - uint64 kstack; // Bottom of kernel stack for this process + uint64 kstack; // Virtual address of kernel stack uint64 sz; // Size of process memory (bytes) pagetable_t pagetable; // Page table struct trapframe *tf; // data page for trampoline.S diff --git a/kernel/spinlock.c b/kernel/spinlock.c index 563532e..f192832 100644 --- a/kernel/spinlock.c +++ b/kernel/spinlock.c @@ -34,7 +34,8 @@ acquire(struct spinlock *lk) // Tell the C compiler and the processor to not move loads or stores // past this point, to ensure that the critical section's memory - // references happen after the lock is acquired. + // references happen strictly after the lock is acquired. + // On RISC-V, this emits a fence instruction. __sync_synchronize(); // Record info about lock acquisition for holding() and debugging. @@ -52,8 +53,10 @@ release(struct spinlock *lk) // Tell the C compiler and the CPU to not move loads or stores // past this point, to ensure that all the stores in the critical - // section are visible to other CPUs before the lock is released. - // On RISC-V, this turns into a fence instruction. + // section are visible to other CPUs before the lock is released, + // and that loads in the critical section occur strictly before + // the lock is released. + // On RISC-V, this emits a fence instruction. __sync_synchronize(); // Release the lock, equivalent to lk->locked = 0. diff --git a/user/alarmtest.c b/user/alarmtest.c index ca3db23..d3746c4 100644 --- a/user/alarmtest.c +++ b/user/alarmtest.c @@ -21,7 +21,7 @@ main(int argc, char *argv[]) { test0(); test1(); - exit(); + exit(0); } volatile static int count; @@ -44,7 +44,7 @@ test0() count = 0; sigalarm(2, periodic); for(i = 0; i < 1000*500000; i++){ - if((i % 250000) == 0) + if((i % 1000000) == 0) write(2, ".", 1); if(count > 0) break; @@ -53,7 +53,7 @@ test0() if(count > 0){ printf("test0 passed\n"); } else { - printf("test0 failed\n"); + printf("\ntest0 failed: the kernel never called the alarm handler\n"); } } @@ -64,6 +64,14 @@ void __attribute__ ((noinline)) foo(int i, int *j) { *j += 1; } +// +// tests that the kernel calls the handler multiple times. +// +// tests that, when the handler returns, it returns to +// the point in the program where the timer interrupt +// occurred, with all registers holding the same values they +// held when the interrupt occurred. +// void test1() { @@ -79,9 +87,19 @@ test1() break; foo(i, &j); } - if(i != j || count < 10){ - // i should equal j - printf("test1 failed\n"); + if(count < 10){ + printf("\ntest1 failed: too few calls to the handler\n"); + exit(1); + } else if(i != j){ + // the loop should have called foo() i times, and foo() should + // have incremented j once per call, so j should equal i. + // once possible source of errors is that the handler may + // return somewhere other than where the timer interrupt + // occurred; another is that that registers may not be + // restored correctly, causing i or j or the address ofj + // to get an incorrect value. + printf("\ntest1 failed: foo() executed fewer times than it was called\n"); + exit(1); } else { printf("test1 passed\n"); } From a52d296814d869f16ced4fb68246223b4a64fa38 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 3 Oct 2019 15:09:31 -0400 Subject: [PATCH 05/31] delete alarmtest from riscv --- user/alarmtest.c | 106 ----------------------------------------------- 1 file changed, 106 deletions(-) delete mode 100644 user/alarmtest.c diff --git a/user/alarmtest.c b/user/alarmtest.c deleted file mode 100644 index d3746c4..0000000 --- a/user/alarmtest.c +++ /dev/null @@ -1,106 +0,0 @@ -// -// test program for the alarm lab. -// you can modify this file for testing, -// but please make sure your kernel -// modifications pass the original -// versions of these tests. -// - -#include "kernel/param.h" -#include "kernel/types.h" -#include "kernel/stat.h" -#include "kernel/riscv.h" -#include "user/user.h" - -void test0(); -void test1(); -void periodic(); - -int -main(int argc, char *argv[]) -{ - test0(); - test1(); - exit(0); -} - -volatile static int count; - -void -periodic() -{ - count = count + 1; - printf("alarm!\n"); - sigreturn(); -} - -// tests whether the kernel calls -// the alarm handler even a single time. -void -test0() -{ - int i; - printf("test0 start\n"); - count = 0; - sigalarm(2, periodic); - for(i = 0; i < 1000*500000; i++){ - if((i % 1000000) == 0) - write(2, ".", 1); - if(count > 0) - break; - } - sigalarm(0, 0); - if(count > 0){ - printf("test0 passed\n"); - } else { - printf("\ntest0 failed: the kernel never called the alarm handler\n"); - } -} - -void __attribute__ ((noinline)) foo(int i, int *j) { - if((i % 2500000) == 0) { - write(2, ".", 1); - } - *j += 1; -} - -// -// tests that the kernel calls the handler multiple times. -// -// tests that, when the handler returns, it returns to -// the point in the program where the timer interrupt -// occurred, with all registers holding the same values they -// held when the interrupt occurred. -// -void -test1() -{ - int i; - int j; - - printf("test1 start\n"); - count = 0; - j = 0; - sigalarm(2, periodic); - for(i = 0; i < 500000000; i++){ - if(count >= 10) - break; - foo(i, &j); - } - if(count < 10){ - printf("\ntest1 failed: too few calls to the handler\n"); - exit(1); - } else if(i != j){ - // the loop should have called foo() i times, and foo() should - // have incremented j once per call, so j should equal i. - // once possible source of errors is that the handler may - // return somewhere other than where the timer interrupt - // occurred; another is that that registers may not be - // restored correctly, causing i or j or the address ofj - // to get an incorrect value. - printf("\ntest1 failed: foo() executed fewer times than it was called\n"); - exit(1); - } else { - printf("test1 passed\n"); - } -} From 8509784d8000d6791a205626e81b03b3f9bf856b Mon Sep 17 00:00:00 2001 From: Anish Athalye Date: Tue, 8 Oct 2019 21:18:54 -0400 Subject: [PATCH 06/31] Add implementations of memcmp and memcpy to ulib This is necessary because gcc may generate calls to memcmp, memset, memcpy, and memmove when compiling with -nostdlib. --- user/ulib.c | 20 ++++++++++++++++++++ user/user.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/user/ulib.c b/user/ulib.c index ddda0f5..8bfba5d 100644 --- a/user/ulib.c +++ b/user/ulib.c @@ -107,3 +107,23 @@ memmove(void *vdst, const void *vsrc, int n) *dst++ = *src++; return vdst; } + +int +memcmp(const void *s1, const void *s2, uint n) +{ + const char *p1 = s1, *p2 = s2; + while (n-- > 0) { + if (*p1 != *p2) { + return *p1 - *p2; + } + p1++; + p2++; + } + return 0; +} + +void * +memcpy(void *dst, const void *src, uint n) +{ + return memmove(dst, src, n); +} diff --git a/user/user.h b/user/user.h index 03af731..b71ecda 100644 --- a/user/user.h +++ b/user/user.h @@ -38,3 +38,5 @@ void* memset(void*, int, uint); void* malloc(uint); void free(void*); int atoi(const char*); +int memcmp(const void *, const void *, uint); +void *memcpy(void *, const void *, uint); From f2df0fa5471c9951ff9a12bea51efbe22afb196e Mon Sep 17 00:00:00 2001 From: Anish Athalye Date: Tue, 8 Oct 2019 21:24:03 -0400 Subject: [PATCH 07/31] Fix ulib's memmove to handle overlap when src 0) - *dst++ = *src++; + if (src > dst) { + while(n-- > 0) + *dst++ = *src++; + } else { + dst += n; + src += n; + while(n-- > 0) + *--dst = *--src; + } return vdst; } From 2821d43cc95b4f9faf79ff94daa5d3a8ea5e7861 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Wed, 16 Oct 2019 12:27:08 -0400 Subject: [PATCH 08/31] nits --- kernel/pipe.c | 4 ++-- kernel/proc.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/pipe.c b/kernel/pipe.c index e358283..c066afb 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -83,7 +83,7 @@ pipewrite(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); for(i = 0; i < n; i++){ while(pi->nwrite == pi->nread + PIPESIZE){ //DOC: pipewrite-full - if(pi->readopen == 0 || myproc()->killed){ + if(pi->readopen == 0 || pr->killed){ release(&pi->lock); return -1; } @@ -108,7 +108,7 @@ piperead(struct pipe *pi, uint64 addr, int n) acquire(&pi->lock); while(pi->nread == pi->nwrite && pi->writeopen){ //DOC: pipe-empty - if(myproc()->killed){ + if(pr->killed){ release(&pi->lock); return -1; } diff --git a/kernel/proc.c b/kernel/proc.c index bbf3af0..ef2ad68 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -116,7 +116,7 @@ found: // Set up new context to start executing at forkret, // which returns to user space. - memset(&p->context, 0, sizeof p->context); + memset(&p->context, 0, sizeof(p->context)); p->context.ra = (uint64)forkret; p->context.sp = p->kstack + PGSIZE; From f2ab0eb644a60f946f35fcb5578fba53720edfa7 Mon Sep 17 00:00:00 2001 From: Anish Athalye Date: Mon, 21 Oct 2019 22:27:18 -0400 Subject: [PATCH 09/31] Clean up linker script This patch does the following: - Add .text.* to the .text section in the output - Add an assertion that the trampoline does not overflow a page - Add the .rodata section - Make .sdata and .sdata.* (which is for small data) be absorbed into the .data section, because we don't need to distinguish between them; this prevents .sdata from appearing in the output - Make the analogous change for .srodata and .sbss - Make all the data sections 16-byte aligned This patch also updates the .editorconfig for *.ld files. --- .editorconfig | 3 +++ kernel/kernel.ld | 36 ++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/.editorconfig b/.editorconfig index cd9dfc3..c47611e 100644 --- a/.editorconfig +++ b/.editorconfig @@ -14,6 +14,9 @@ indent_size = 2 [*.S] indent_size = 8 +[*.ld] +indent_size = 2 + [Makefile] indent_style = tab indent_size = 8 diff --git a/kernel/kernel.ld b/kernel/kernel.ld index acc3c8e..ee04f22 100644 --- a/kernel/kernel.ld +++ b/kernel/kernel.ld @@ -8,25 +8,37 @@ SECTIONS * where qemu's -kernel jumps. */ . = 0x80000000; - .text : - { - *(.text) + + .text : { + *(.text .text.*) . = ALIGN(0x1000); + _trampoline = .; *(trampsec) + . = ALIGN(0x1000); + ASSERT(. - _trampoline == 0x1000, "error: trampoline larger than one page"); + PROVIDE(etext = .); } - . = ALIGN(0x1000); - PROVIDE(etext = .); + .rodata : { + . = ALIGN(16); + *(.srodata .srodata.*) /* do not need to distinguish this from .rodata */ + . = ALIGN(16); + *(.rodata .rodata.*) + } - /* - * make sure end is after data and bss. - */ .data : { - *(.data) + . = ALIGN(16); + *(.sdata .sdata.*) /* do not need to distinguish this from .data */ + . = ALIGN(16); + *(.data .data.*) } + .bss : { - *(.bss) - *(.sbss*) - PROVIDE(end = .); + . = ALIGN(16); + *(.sbss .sbss.*) /* do not need to distinguish this from .bss */ + . = ALIGN(16); + *(.bss .bss.*) } + + PROVIDE(end = .); } From d9160fb4b98e3ce04d3928c1fbd2ec26b3cc746a Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Sun, 27 Oct 2019 08:03:19 -0400 Subject: [PATCH 10/31] nits --- Makefile | 2 +- kernel/riscv.h | 1 - kernel/spinlock.c | 7 +++---- kernel/start.c | 1 + kernel/trap.c | 7 +++++-- user/usertests.c | 4 ++-- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index ba7a6fb..7229d5e 100644 --- a/Makefile +++ b/Makefile @@ -104,7 +104,7 @@ $U/_forktest: $U/forktest.o $(ULIB) $(LD) $(LDFLAGS) -N -e main -Ttext 0 -o $U/_forktest $U/forktest.o $U/ulib.o $U/usys.o $(OBJDUMP) -S $U/_forktest > $U/forktest.asm -mkfs/mkfs: mkfs/mkfs.c $K/fs.h +mkfs/mkfs: mkfs/mkfs.c $K/fs.h $K/param.h gcc -Werror -Wall -I. -o mkfs/mkfs mkfs/mkfs.c # Prevent deletion of intermediate files, e.g. cat.o, after first build, so diff --git a/kernel/riscv.h b/kernel/riscv.h index f46ba59..0aec003 100644 --- a/kernel/riscv.h +++ b/kernel/riscv.h @@ -261,7 +261,6 @@ r_time() static inline void intr_on() { - w_sie(r_sie() | SIE_SEIE | SIE_STIE | SIE_SSIE); w_sstatus(r_sstatus() | SSTATUS_SIE); } diff --git a/kernel/spinlock.c b/kernel/spinlock.c index f192832..9840302 100644 --- a/kernel/spinlock.c +++ b/kernel/spinlock.c @@ -72,13 +72,12 @@ release(struct spinlock *lk) } // Check whether this cpu is holding the lock. +// Interrupts must be off. int holding(struct spinlock *lk) { int r; - push_off(); r = (lk->locked && lk->cpu == mycpu()); - pop_off(); return r; } @@ -103,9 +102,9 @@ pop_off(void) struct cpu *c = mycpu(); if(intr_get()) panic("pop_off - interruptible"); - c->noff -= 1; - if(c->noff < 0) + if(c->noff < 1) panic("pop_off"); + c->noff -= 1; if(c->noff == 0 && c->intena) intr_on(); } diff --git a/kernel/start.c b/kernel/start.c index 203c5e6..4eb6c2d 100644 --- a/kernel/start.c +++ b/kernel/start.c @@ -36,6 +36,7 @@ start() // delegate all interrupts and exceptions to supervisor mode. w_medeleg(0xffff); w_mideleg(0xffff); + w_sie(r_sie() | SIE_SEIE | SIE_STIE | SIE_SSIE); // ask for clock interrupts. timerinit(); diff --git a/kernel/trap.c b/kernel/trap.c index ca732f2..5e11e4b 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -129,7 +129,6 @@ usertrapret(void) // interrupts and exceptions from kernel code go here via kernelvec, // on whatever the current kernel stack is. -// must be 4-byte aligned to fit in stvec. void kerneltrap() { @@ -189,9 +188,13 @@ devintr() uartintr(); } else if(irq == VIRTIO0_IRQ){ virtio_disk_intr(); + } else if(irq){ + printf("unexpected interrupt irq=%d\n", irq); } - plic_complete(irq); + if(irq) + plic_complete(irq); + return 1; } else if(scause == 0x8000000000000001L){ // software interrupt from a machine-mode timer interrupt, diff --git a/user/usertests.c b/user/usertests.c index db9f680..eb10ee2 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -2105,9 +2105,9 @@ run(void f(char *), char *s) { } else { wait(&xstatus); if(xstatus != 0) - printf("FAILED\n", s); + printf("FAILED\n"); else - printf("OK\n", s); + printf("OK\n"); return xstatus == 0; } } From e7ffb74ad1e4c4e8a4a5e62968f52499dc0c7079 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Sun, 27 Oct 2019 13:36:46 -0400 Subject: [PATCH 11/31] fix a potential memory leak --- kernel/proc.c | 3 +-- kernel/vm.c | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index ef2ad68..0cb5afe 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -176,8 +176,7 @@ proc_freepagetable(pagetable_t pagetable, uint64 sz) { uvmunmap(pagetable, TRAMPOLINE, PGSIZE, 0); uvmunmap(pagetable, TRAPFRAME, PGSIZE, 0); - if(sz > 0) - uvmfree(pagetable, sz); + uvmfree(pagetable, sz); } // a user program that calls exec("/init") diff --git a/kernel/vm.c b/kernel/vm.c index c5da0c1..3004bb3 100644 --- a/kernel/vm.c +++ b/kernel/vm.c @@ -301,7 +301,8 @@ freewalk(pagetable_t pagetable) void uvmfree(pagetable_t pagetable, uint64 sz) { - uvmunmap(pagetable, 0, sz, 1); + if(sz > 0) + uvmunmap(pagetable, 0, sz, 1); freewalk(pagetable); } From 9de9211b1158bfbe169c5099dae11432d5950105 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Mon, 28 Oct 2019 05:58:28 -0400 Subject: [PATCH 12/31] usertests -c to repeat tests forever detect memory leaks no more "already ran user tests" --- user/usertests.c | 96 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 80 insertions(+), 16 deletions(-) diff --git a/user/usertests.c b/user/usertests.c index eb10ee2..8d1d06a 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -1418,6 +1418,14 @@ fourteen(char *s) printf("%s: mkdir 12345678901234/123456789012345 succeeded!\n", s); exit(1); } + + // clean up + unlink("123456789012345/12345678901234"); + unlink("12345678901234/12345678901234"); + unlink("12345678901234/12345678901234/12345678901234"); + unlink("123456789012345/123456789012345/123456789012345"); + unlink("12345678901234/123456789012345"); + unlink("12345678901234"); } void @@ -1512,7 +1520,8 @@ dirfile(char *s) close(fd); } -// test that iput() is called at the end of _namei() +// test that iput() is called at the end of _namei(). +// also tests empty file names. void iref(char *s) { @@ -1539,6 +1548,12 @@ iref(char *s) unlink("xx"); } + // clean up + for(i = 0; i < NINODE + 1; i++){ + chdir(".."); + unlink("irefd"); + } + chdir("/"); } @@ -2087,13 +2102,32 @@ badarg(char *s) exit(0); } +// +// use sbrk() to count how many free physical memory pages there are. +// +int +countfree() +{ + uint64 sz0 = (uint64)sbrk(0); + int n = 0; + + while(1){ + if((uint64)sbrk(4096) == 0xffffffffffffffff){ + break; + } + n += 1; + } + sbrk(-((uint64)sbrk(0) - sz0)); + return n; +} + // run each test in its own process. run returns 1 if child's exit() // indicates success. int run(void f(char *), char *s) { int pid; int xstatus; - + printf("test %s: ", s); if((pid = fork()) < 0) { printf("runtest: fork error\n"); @@ -2115,9 +2149,16 @@ run(void f(char *), char *s) { int main(int argc, char *argv[]) { - char *n = 0; - if(argc > 1) { - n = argv[1]; + int continuous = 0; + char *justone = 0; + + if(argc == 2 && strcmp(argv[1], "-c") == 0){ + continuous = 1; + } else if(argc == 2 && argv[1][0] != '-'){ + justone = argv[1]; + } else if(argc > 1){ + printf("Usage: usertests [-c] [testname]\n"); + exit(1); } struct test { @@ -2173,25 +2214,48 @@ main(int argc, char *argv[]) {bigdir, "bigdir"}, // slow { 0, 0}, }; - - printf("usertests starting\n"); - if(open("usertests.ran", 0) >= 0){ - printf("already ran user tests -- rebuild fs.img (rm fs.img; make fs.img)\n"); - exit(1); + if(continuous){ + printf("continuous usertests starting\n"); + while(1){ + int fail = 0; + int free0 = countfree(); + for (struct test *t = tests; t->s != 0; t++) { + if(!run(t->f, t->s)){ + fail = 1; + break; + } + } + if(fail){ + printf("SOME TESTS FAILED\n"); + exit(1); + } + int free1 = countfree(); + if(free1 < free0){ + printf("FAILED -- lost some free pages\n"); + exit(1); + } + } } - close(open("usertests.ran", O_CREATE)); + printf("usertests starting\n"); + int free0 = countfree(); int fail = 0; for (struct test *t = tests; t->s != 0; t++) { - if((n == 0) || strcmp(t->s, n) == 0) { + if((justone == 0) || strcmp(t->s, justone) == 0) { if(!run(t->f, t->s)) fail = 1; } } - if(!fail) - printf("ALL TESTS PASSED\n"); - else + + if(fail){ printf("SOME TESTS FAILED\n"); - exit(1); // not reached. + exit(1); + } else if(countfree() < free0){ + printf("FAILED -- lost some free pages\n"); + exit(1); + } else { + printf("ALL TESTS PASSED\n"); + exit(0); + } } From 028af2764622d489583cd88935cd1d2a7fbe8248 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Tue, 29 Oct 2019 04:32:55 -0400 Subject: [PATCH 13/31] mention LRU list a bit more in comments. --- kernel/bio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kernel/bio.c b/kernel/bio.c index a1074f2..60d91a6 100644 --- a/kernel/bio.c +++ b/kernel/bio.c @@ -28,7 +28,8 @@ struct { struct buf buf[NBUF]; // Linked list of all buffers, through prev/next. - // head.next is most recently used. + // Sorted by how recently the buffer was used. + // head.next is most recent, head.prev is least. struct buf head; } bcache; @@ -71,7 +72,8 @@ bget(uint dev, uint blockno) } } - // Not cached; recycle an unused buffer. + // Not cached. + // Recycle the least recently used (LRU) unused buffer. for(b = bcache.head.prev; b != &bcache.head; b = b->prev){ if(b->refcnt == 0) { b->dev = dev; @@ -110,7 +112,7 @@ bwrite(struct buf *b) } // Release a locked buffer. -// Move to the head of the MRU list. +// Move to the head of the most-recently-used list. void brelse(struct buf *b) { From 16b3b63f06c1ea17da484aeebea4a57fb2a6e44a Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Wed, 6 Nov 2019 11:18:43 -0500 Subject: [PATCH 14/31] grind: run parallel system calls forever --- Makefile | 1 + user/grind.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++ user/usertests.c | 14 +++-- 3 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 user/grind.c diff --git a/Makefile b/Makefile index 7229d5e..46fd956 100644 --- a/Makefile +++ b/Makefile @@ -127,6 +127,7 @@ UPROGS=\ $U/_sh\ $U/_stressfs\ $U/_usertests\ + $U/_grind\ $U/_wc\ $U/_zombie\ diff --git a/user/grind.c b/user/grind.c new file mode 100644 index 0000000..97a8a93 --- /dev/null +++ b/user/grind.c @@ -0,0 +1,160 @@ +// +// run random system calls in parallel forever. +// + +#include "kernel/param.h" +#include "kernel/types.h" +#include "kernel/stat.h" +#include "user/user.h" +#include "kernel/fs.h" +#include "kernel/fcntl.h" +#include "kernel/syscall.h" +#include "kernel/memlayout.h" +#include "kernel/riscv.h" + +// from FreeBSD. +int +do_rand(unsigned long *ctx) +{ +/* + * Compute x = (7^5 * x) mod (2^31 - 1) + * without overflowing 31 bits: + * (2^31 - 1) = 127773 * (7^5) + 2836 + * From "Random number generators: good ones are hard to find", + * Park and Miller, Communications of the ACM, vol. 31, no. 10, + * October 1988, p. 1195. + */ + long hi, lo, x; + + /* Transform to [1, 0x7ffffffe] range. */ + x = (*ctx % 0x7ffffffe) + 1; + hi = x / 127773; + lo = x % 127773; + x = 16807 * lo - 2836 * hi; + if (x < 0) + x += 0x7fffffff; + /* Transform to [0, 0x7ffffffd] range. */ + x--; + *ctx = x; + return (x); +} + +unsigned long rand_next = 1; + +int +rand(void) +{ + return (do_rand(&rand_next)); +} + +void +go() +{ + int fd = -1; + static char buf[999]; + char *break0 = sbrk(0); + + while(1){ + int what = rand() % 20; + if(what == 1){ + close(open("a", O_CREATE|O_RDWR)); + } else if(what == 2){ + close(open("b", O_CREATE|O_RDWR)); + } else if(what == 3){ + unlink("a"); + } else if(what == 4){ + unlink("b"); + } else if(what == 5){ + close(fd); + fd = open("a", O_CREATE|O_RDWR); + } else if(what == 6){ + close(fd); + fd = open("b", O_CREATE|O_RDWR); + } else if(what == 7){ + write(fd, buf, sizeof(buf)); + } else if(what == 8){ + read(fd, buf, sizeof(buf)); + } else if(what == 9){ + mkdir("a"); + } else if(what == 10){ + mkdir("b"); + } else if(what == 11){ + unlink("b"); + link("a", "b"); + } else if(what == 12){ + unlink("a"); + link("b", "a"); + } else if(what == 13){ + int pid = fork(); + if(pid == 0){ + exit(0); + } + wait(0); + } else if(what == 14){ + int pid = fork(); + if(pid == 0){ + fork(); + fork(); + exit(0); + } + wait(0); + } else if(what == 15){ + sbrk(6011); + } else if(what == 16){ + if(sbrk(0) > break0) + sbrk(-(sbrk(0) - break0)); + } else if(what == 17){ + int pid = fork(); + if(pid == 0){ + close(open("a", O_CREATE|O_RDWR)); + exit(0); + } + kill(pid); + wait(0); + } else if(what == 18){ + int pid = fork(); + if(pid == 0){ + kill(getpid()); + exit(0); + } + wait(0); + } + } +} + +int +main() +{ + int pid1 = fork(); + if(pid1 < 0){ + printf("grind: fork failed\n"); + exit(1); + } + if(pid1 == 0){ + rand_next = 31; + go(); + exit(0); + } + + int pid2 = fork(); + if(pid2 < 0){ + printf("grind: fork failed\n"); + exit(1); + } + if(pid2 == 0){ + rand_next = 7177; + go(); + exit(0); + } + + int st1 = -1; + wait(&st1); + if(st1 != 0){ + kill(pid1); + kill(pid2); + } + int st2 = -1; + wait(&st2); + + exit(0); +} diff --git a/user/usertests.c b/user/usertests.c index 8d1d06a..9aa0ed4 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -1038,11 +1038,15 @@ concreate(char *s) close(open(file, 0)); close(open(file, 0)); close(open(file, 0)); + close(open(file, 0)); + close(open(file, 0)); } else { unlink(file); unlink(file); unlink(file); unlink(file); + unlink(file); + unlink(file); } if(pid == 0) exit(0); @@ -1106,7 +1110,7 @@ bigdir(char *s) name[2] = '0' + (i % 64); name[3] = '\0'; if(link("bd", name) != 0){ - printf("%s: bigdir link failed\n", s); + printf("%s: bigdir link(bd, %s) failed\n", s, name); exit(1); } } @@ -1335,8 +1339,8 @@ bigfile(char *s) enum { N = 20, SZ=600 }; int fd, i, total, cc; - unlink("bigfile"); - fd = open("bigfile", O_CREATE | O_RDWR); + unlink("bigfile.dat"); + fd = open("bigfile.dat", O_CREATE | O_RDWR); if(fd < 0){ printf("%s: cannot create bigfile", s); exit(1); @@ -1350,7 +1354,7 @@ bigfile(char *s) } close(fd); - fd = open("bigfile", 0); + fd = open("bigfile.dat", 0); if(fd < 0){ printf("%s: cannot open bigfile\n", s); exit(1); @@ -1379,7 +1383,7 @@ bigfile(char *s) printf("%s: read bigfile wrong total\n", s); exit(1); } - unlink("bigfile"); + unlink("bigfile.dat"); } void From 20f1dd940964d7e01cf8c8d9b1a5b751840b7f3b Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 7 Nov 2019 06:44:23 -0500 Subject: [PATCH 15/31] more grind --- Makefile | 2 +- user/grind.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 46fd956..574911b 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ TOOLPREFIX := $(shell if riscv64-unknown-elf-objdump -i 2>&1 | grep 'elf64-big' elif riscv64-linux-gnu-objdump -i 2>&1 | grep 'elf64-big' >/dev/null 2>&1; \ then echo 'riscv64-linux-gnu-'; \ else echo "***" 1>&2; \ - echo "*** Error: Couldn't find an riscv64 version of GCC/binutils." 1>&2; \ + echo "*** Error: Couldn't find a riscv64 version of GCC/binutils." 1>&2; \ echo "*** To turn off this error, run 'gmake TOOLPREFIX= ...'." 1>&2; \ echo "***" 1>&2; exit 1; fi) endif diff --git a/user/grind.c b/user/grind.c index 97a8a93..a01271e 100644 --- a/user/grind.c +++ b/user/grind.c @@ -48,13 +48,17 @@ rand(void) } void -go() +go(int which_child) { int fd = -1; static char buf[999]; char *break0 = sbrk(0); + uint64 iters = 0; while(1){ + iters++; + if((iters % 500) == 0) + write(1, which_child?"B":"A", 1); int what = rand() % 20; if(what == 1){ close(open("a", O_CREATE|O_RDWR)); @@ -76,8 +80,12 @@ go() read(fd, buf, sizeof(buf)); } else if(what == 9){ mkdir("a"); + close(open("a/a", O_CREATE|O_RDWR)); + unlink("a/a"); } else if(what == 10){ mkdir("b"); + close(open("b/b", O_CREATE|O_RDWR)); + unlink("b/b"); } else if(what == 11){ unlink("b"); link("a", "b"); @@ -88,6 +96,9 @@ go() int pid = fork(); if(pid == 0){ exit(0); + } else if(pid < 0){ + printf("grind: fork failed\n"); + exit(1); } wait(0); } else if(what == 14){ @@ -96,6 +107,9 @@ go() fork(); fork(); exit(0); + } else if(pid < 0){ + printf("grind: fork failed\n"); + exit(1); } wait(0); } else if(what == 15){ @@ -108,6 +122,9 @@ go() if(pid == 0){ close(open("a", O_CREATE|O_RDWR)); exit(0); + } else if(pid < 0){ + printf("grind: fork failed\n"); + exit(1); } kill(pid); wait(0); @@ -116,8 +133,34 @@ go() if(pid == 0){ kill(getpid()); exit(0); + } else if(pid < 0){ + printf("grind: fork failed\n"); + exit(1); } wait(0); + } else if(what == 19){ + int fds[2]; + if(pipe(fds) < 0){ + printf("grind: pipe failed\n"); + exit(1); + } + int pid = fork(); + if(pid == 0){ + fork(); + fork(); + if(write(fds[1], "x", 1) != 1) + printf("grind: pipe write failed\n"); + char c; + if(read(fds[0], &c, 1) != 1) + printf("grind: pipe read failed\n"); + exit(0); + } else if(pid < 0){ + printf("grind: fork failed\n"); + exit(1); + } + close(fds[0]); + close(fds[1]); + wait(0); } } } @@ -125,6 +168,9 @@ go() int main() { + unlink("a"); + unlink("b"); + int pid1 = fork(); if(pid1 < 0){ printf("grind: fork failed\n"); @@ -132,7 +178,7 @@ main() } if(pid1 == 0){ rand_next = 31; - go(); + go(0); exit(0); } @@ -143,7 +189,7 @@ main() } if(pid2 == 0){ rand_next = 7177; - go(); + go(1); exit(0); } From 897f6f34ddf2b45f7ef0e2474691f28cb6c9c0bd Mon Sep 17 00:00:00 2001 From: rtm Date: Thu, 7 Nov 2019 06:43:38 -0500 Subject: [PATCH 16/31] yet another toolchain name --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 574911b..7bfee58 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,8 @@ TOOLPREFIX := $(shell if riscv64-unknown-elf-objdump -i 2>&1 | grep 'elf64-big' then echo 'riscv64-unknown-elf-'; \ elif riscv64-linux-gnu-objdump -i 2>&1 | grep 'elf64-big' >/dev/null 2>&1; \ then echo 'riscv64-linux-gnu-'; \ + elif riscv64-unknown-linux-gnu-objdump -i 2>&1 | grep 'elf64-big' >/dev/null 2>&1; \ + then echo 'riscv64-unknown-linux-gnu-'; \ else echo "***" 1>&2; \ echo "*** Error: Couldn't find a riscv64 version of GCC/binutils." 1>&2; \ echo "*** To turn off this error, run 'gmake TOOLPREFIX= ...'." 1>&2; \ From b62d4d412bf36eb445dd05cf80762ba8837de6ce Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 7 Nov 2019 09:46:20 -0500 Subject: [PATCH 17/31] more grind --- user/cat.c | 8 +-- user/grind.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 145 insertions(+), 18 deletions(-) diff --git a/user/cat.c b/user/cat.c index 36939d8..598f005 100644 --- a/user/cat.c +++ b/user/cat.c @@ -11,12 +11,12 @@ cat(int fd) while((n = read(fd, buf, sizeof(buf))) > 0) { if (write(1, buf, n) != n) { - printf("cat: write error\n"); + fprintf(2, "cat: write error\n"); exit(1); } } if(n < 0){ - printf("cat: read error\n"); + fprintf(2, "cat: read error\n"); exit(1); } } @@ -28,12 +28,12 @@ main(int argc, char *argv[]) if(argc <= 1){ cat(0); - exit(1); + exit(0); } for(i = 1; i < argc; i++){ if((fd = open(argv[i], 0)) < 0){ - printf("cat: cannot open %s\n", argv[i]); + fprintf(2, "cat: cannot open %s\n", argv[i]); exit(1); } cat(fd); diff --git a/user/grind.c b/user/grind.c index a01271e..6203e57 100644 --- a/user/grind.c +++ b/user/grind.c @@ -54,44 +54,56 @@ go(int which_child) static char buf[999]; char *break0 = sbrk(0); uint64 iters = 0; + + mkdir("grindir"); + if(chdir("grindir") != 0){ + printf("chdir grindir failed\n"); + exit(1); + } + chdir("/"); while(1){ iters++; if((iters % 500) == 0) write(1, which_child?"B":"A", 1); - int what = rand() % 20; + int what = rand() % 23; if(what == 1){ - close(open("a", O_CREATE|O_RDWR)); + close(open("grindir/../a", O_CREATE|O_RDWR)); } else if(what == 2){ - close(open("b", O_CREATE|O_RDWR)); + close(open("grindir/../grindir/../b", O_CREATE|O_RDWR)); } else if(what == 3){ - unlink("a"); + unlink("grindir/../a"); } else if(what == 4){ - unlink("b"); + if(chdir("grindir") != 0){ + printf("chdir grindir failed\n"); + exit(1); + } + unlink("../b"); + chdir("/"); } else if(what == 5){ close(fd); - fd = open("a", O_CREATE|O_RDWR); + fd = open("/grindir/../a", O_CREATE|O_RDWR); } else if(what == 6){ close(fd); - fd = open("b", O_CREATE|O_RDWR); + fd = open("/./grindir/./../b", O_CREATE|O_RDWR); } else if(what == 7){ write(fd, buf, sizeof(buf)); } else if(what == 8){ read(fd, buf, sizeof(buf)); } else if(what == 9){ - mkdir("a"); - close(open("a/a", O_CREATE|O_RDWR)); + mkdir("grindir/../a"); + close(open("a/../a/./a", O_CREATE|O_RDWR)); unlink("a/a"); } else if(what == 10){ - mkdir("b"); - close(open("b/b", O_CREATE|O_RDWR)); + mkdir("/../b"); + close(open("grindir/../b/b", O_CREATE|O_RDWR)); unlink("b/b"); } else if(what == 11){ unlink("b"); - link("a", "b"); + link("../grindir/./../a", "../b"); } else if(what == 12){ - unlink("a"); - link("b", "a"); + unlink("../grindir/../a"); + link(".././b", "/grindir/../a"); } else if(what == 13){ int pid = fork(); if(pid == 0){ @@ -126,6 +138,10 @@ go(int which_child) printf("grind: fork failed\n"); exit(1); } + if(chdir("../grindir/..") != 0){ + printf("chdir failed\n"); + exit(1); + } kill(pid); wait(0); } else if(what == 18){ @@ -161,6 +177,117 @@ go(int which_child) close(fds[0]); close(fds[1]); wait(0); + } else if(what == 20){ + int pid = fork(); + if(pid == 0){ + unlink("a"); + mkdir("a"); + chdir("a"); + unlink("../a"); + fd = open("x", O_CREATE|O_RDWR); + unlink("x"); + exit(0); + } else if(pid < 0){ + printf("fork failed\n"); + exit(1); + } + wait(0); + } else if(what == 21){ + unlink("c"); + // should always succeed. check that there are free i-nodes, + // file descriptors, blocks. + int fd1 = open("c", O_CREATE|O_RDWR); + if(fd1 < 0){ + printf("create c failed\n"); + exit(1); + } + if(write(fd1, "x", 1) != 1){ + printf("write c failed\n"); + exit(1); + } + struct stat st; + if(fstat(fd1, &st) != 0){ + printf("fstat failed\n"); + exit(1); + } + if(st.size != 1){ + printf("fstat reports wrong size %d\n", (int)st.size); + exit(1); + } + if(st.ino > 50){ + printf("fstat reports crazy i-number %d\n", st.ino); + exit(1); + } + close(fd1); + unlink("c"); + } else if(what == 22){ + // echo hi | cat + int aa[2], bb[2]; + if(pipe(aa) < 0){ + fprintf(2, "pipe failed\n"); + exit(1); + } + if(pipe(bb) < 0){ + fprintf(2, "pipe failed\n"); + exit(1); + } + int pid1 = fork(); + if(pid1 == 0){ + close(bb[0]); + close(bb[1]); + close(aa[0]); + close(1); + if(dup(aa[1]) != 1){ + fprintf(2, "dup failed\n"); + exit(1); + } + close(aa[1]); + char *args[3] = { "echo", "hi", 0 }; + exec("grindir/../echo", args); + fprintf(2, "echo: not found\n"); + exit(2); + } else if(pid1 < 0){ + fprintf(2, "fork failed\n"); + exit(3); + } + int pid2 = fork(); + if(pid2 == 0){ + close(aa[1]); + close(bb[0]); + close(0); + if(dup(aa[0]) != 0){ + fprintf(2, "dup failed\n"); + exit(4); + } + close(aa[0]); + close(1); + if(dup(bb[1]) != 1){ + fprintf(2, "dup failed\n"); + exit(5); + } + close(bb[1]); + char *args[2] = { "cat", 0 }; + exec("/cat", args); + fprintf(2, "cat: not found\n"); + exit(6); + } else if(pid2 < 0){ + fprintf(2, "fork failed\n"); + exit(7); + } + close(aa[0]); + close(aa[1]); + close(bb[1]); + char buf[3] = { 0, 0, 0 }; + read(bb[0], buf+0, 1); + read(bb[0], buf+1, 1); + close(bb[0]); + int st1, st2; + wait(&st1); + wait(&st2); + if(st1 != 0 || st2 != 0 || strcmp(buf, "hi") != 0){ + printf("exec pipeline failed %d %d \"%s\"\n", st1, st2, buf); + exit(1); + } } } } From 672217ae2a3d68b73b2229ab368979ef7790e28a Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 8 Nov 2019 13:21:06 -0500 Subject: [PATCH 18/31] allow more files --- user/grind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/grind.c b/user/grind.c index 6203e57..14e2aae 100644 --- a/user/grind.c +++ b/user/grind.c @@ -214,7 +214,7 @@ go(int which_child) printf("fstat reports wrong size %d\n", (int)st.size); exit(1); } - if(st.ino > 50){ + if(st.ino > 200){ printf("fstat reports crazy i-number %d\n", st.ino); exit(1); } From af9eb9114c2f8700d4315eaa1e2d637c2aaaf210 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 16 Jul 2020 11:38:08 -0400 Subject: [PATCH 19/31] make "echo hello > x" truncate file x. --- kernel/defs.h | 1 + kernel/fcntl.h | 1 + kernel/fs.c | 10 ++-- kernel/sysfile.c | 4 ++ user/sh.c | 2 +- user/usertests.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 148 insertions(+), 8 deletions(-) diff --git a/kernel/defs.h b/kernel/defs.h index 9c5f643..f33f1f6 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -52,6 +52,7 @@ struct inode* nameiparent(char*, char*); int readi(struct inode*, int, uint64, uint, uint); void stati(struct inode*, struct stat*); int writei(struct inode*, int, uint64, uint, uint); +void itrunc(struct inode*); // ramdisk.c void ramdiskinit(void); diff --git a/kernel/fcntl.h b/kernel/fcntl.h index d565483..44861b9 100644 --- a/kernel/fcntl.h +++ b/kernel/fcntl.h @@ -2,3 +2,4 @@ #define O_WRONLY 0x001 #define O_RDWR 0x002 #define O_CREATE 0x200 +#define O_TRUNC 0x400 diff --git a/kernel/fs.c b/kernel/fs.c index 53586d5..e33ec30 100644 --- a/kernel/fs.c +++ b/kernel/fs.c @@ -22,7 +22,6 @@ #include "file.h" #define min(a, b) ((a) < (b) ? (a) : (b)) -static void itrunc(struct inode*); // there should be one superblock per disk device, but we run with // only one device struct superblock sb; @@ -406,11 +405,8 @@ bmap(struct inode *ip, uint bn) } // Truncate inode (discard contents). -// Only called when the inode has no links -// to it (no directory entries referring to it) -// and has no in-memory reference to it (is -// not an open file or current directory). -static void +// Caller must hold ip->lock. +void itrunc(struct inode *ip) { int i, j; @@ -463,7 +459,7 @@ readi(struct inode *ip, int user_dst, uint64 dst, uint off, uint n) struct buf *bp; if(off > ip->size || off + n < off) - return -1; + return 0; if(off + n > ip->size) n = ip->size - off; diff --git a/kernel/sysfile.c b/kernel/sysfile.c index 7768d20..015c942 100644 --- a/kernel/sysfile.c +++ b/kernel/sysfile.c @@ -341,6 +341,10 @@ sys_open(void) f->readable = !(omode & O_WRONLY); f->writable = (omode & O_WRONLY) || (omode & O_RDWR); + if((omode & O_TRUNC) && ip->type == T_FILE){ + itrunc(ip); + } + iunlock(ip); end_op(); diff --git a/user/sh.c b/user/sh.c index a593bc0..83dd513 100644 --- a/user/sh.c +++ b/user/sh.c @@ -386,7 +386,7 @@ parseredirs(struct cmd *cmd, char **ps, char *es) cmd = redircmd(cmd, q, eq, O_RDONLY, 0); break; case '>': - cmd = redircmd(cmd, q, eq, O_WRONLY|O_CREATE, 1); + cmd = redircmd(cmd, q, eq, O_WRONLY|O_CREATE|O_TRUNC, 1); break; case '+': // >> cmd = redircmd(cmd, q, eq, O_WRONLY|O_CREATE, 1); diff --git a/user/usertests.c b/user/usertests.c index 9aa0ed4..f83a060 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -22,6 +22,141 @@ char buf[BUFSZ]; char name[3]; +// test O_TRUNC. +void +truncate1(char *s) +{ + char buf[32]; + + unlink("truncfile"); + int fd1 = open("truncfile", O_CREATE|O_WRONLY|O_TRUNC); + write(fd1, "abcd", 4); + close(fd1); + + int fd2 = open("truncfile", O_RDONLY); + int n = read(fd2, buf, sizeof(buf)); + if(n != 4){ + printf("%s: read %d bytes, wanted 4\n", s, n); + exit(1); + } + + fd1 = open("truncfile", O_WRONLY|O_TRUNC); + + int fd3 = open("truncfile", O_RDONLY); + n = read(fd3, buf, sizeof(buf)); + if(n != 0){ + printf("aaa fd3=%d\n", fd3); + printf("%s: read %d bytes, wanted 0\n", s, n); + exit(1); + } + + n = read(fd2, buf, sizeof(buf)); + if(n != 0){ + printf("bbb fd2=%d\n", fd2); + printf("%s: read %d bytes, wanted 0\n", s, n); + exit(1); + } + + write(fd1, "abcdef", 6); + + n = read(fd3, buf, sizeof(buf)); + if(n != 6){ + printf("%s: read %d bytes, wanted 6\n", s, n); + exit(1); + } + + n = read(fd2, buf, sizeof(buf)); + if(n != 2){ + printf("%s: read %d bytes, wanted 2\n", s, n); + exit(1); + } + + unlink("truncfile"); + + close(fd1); + close(fd2); + close(fd3); +} + +// write to an open FD whose file has just been truncated. +// this causes a write at an offset beyond the end of the file. +// such writes fail on xv6 (unlike POSIX) but at least +// they don't crash. +void +truncate2(char *s) +{ + unlink("truncfile"); + + int fd1 = open("truncfile", O_CREATE|O_TRUNC|O_WRONLY); + write(fd1, "abcd", 4); + + int fd2 = open("truncfile", O_TRUNC|O_WRONLY); + + int n = write(fd1, "x", 1); + if(n != -1){ + printf("%s: write returned %d, expected -1\n", s, n); + exit(1); + } + + unlink("truncfile"); + close(fd1); + close(fd2); +} + +void +truncate3(char *s) +{ + int pid, xstatus; + + close(open("truncfile", O_CREATE|O_TRUNC|O_WRONLY)); + + pid = fork(); + if(pid < 0){ + printf("%s: fork failed\n", s); + exit(1); + } + + if(pid == 0){ + for(int i = 0; i < 100; i++){ + char buf[32]; + int fd = open("truncfile", O_WRONLY); + if(fd < 0){ + printf("%s: open failed\n", s); + exit(1); + } + int n = write(fd, "1234567890", 10); + if(n != 10){ + printf("%s: write got %d, expected 10\n", s, n); + exit(1); + } + close(fd); + fd = open("truncfile", O_RDONLY); + read(fd, buf, sizeof(buf)); + close(fd); + } + exit(0); + } + + for(int i = 0; i < 150; i++){ + int fd = open("truncfile", O_CREATE|O_WRONLY|O_TRUNC); + if(fd < 0){ + printf("%s: open failed\n", s); + exit(1); + } + int n = write(fd, "xxx", 3); + if(n != 3){ + printf("%s: write got %d, expected 3\n", s, n); + exit(1); + } + close(fd); + } + + wait(&xstatus); + unlink("truncfile"); + exit(xstatus); +} + + // does chdir() call iput(p->cwd) in a transaction? void iputtest(char *s) @@ -2169,6 +2304,9 @@ main(int argc, char *argv[]) void (*f)(char *); char *s; } tests[] = { + {truncate1, "truncate1"}, + {truncate2, "truncate2"}, + {truncate3, "truncate3"}, {reparent2, "reparent2"}, {pgbug, "pgbug" }, {sbrkbugs, "sbrkbugs" }, From 82981fab6bbd36dc0ff24201a11d365c4a2c6b1d Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 17 Jul 2020 16:29:43 -0400 Subject: [PATCH 20/31] drop QEMUEXTRA --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7bfee58..96f1c14 100644 --- a/Makefile +++ b/Makefile @@ -156,9 +156,9 @@ ifndef CPUS CPUS := 3 endif -QEMUEXTRA = -drive file=fs1.img,if=none,format=raw,id=x1 -device virtio-blk-device,drive=x1,bus=virtio-mmio-bus.1 QEMUOPTS = -machine virt -bios none -kernel $K/kernel -m 128M -smp $(CPUS) -nographic -QEMUOPTS += -drive file=fs.img,if=none,format=raw,id=x0 -device virtio-blk-device,drive=x0,bus=virtio-mmio-bus.0 +QEMUOPTS += -drive file=fs.img,if=none,format=raw,id=x0 +QEMUOPTS += -device virtio-blk-device,drive=x0,bus=virtio-mmio-bus.0 qemu: $K/kernel fs.img $(QEMU) $(QEMUOPTS) From 5494c9170587b17c9d749e19753fa3e7fb6958b9 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 17 Jul 2020 16:29:52 -0400 Subject: [PATCH 21/31] rename p->tf to p->trapframe, for consistency with p->context --- kernel/exec.c | 6 +++--- kernel/memlayout.h | 2 +- kernel/proc.c | 18 +++++++++--------- kernel/proc.h | 2 +- kernel/syscall.c | 18 +++++++++--------- kernel/trap.c | 14 +++++++------- 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/kernel/exec.c b/kernel/exec.c index 6a51499..1077ac0 100644 --- a/kernel/exec.c +++ b/kernel/exec.c @@ -97,7 +97,7 @@ exec(char *path, char **argv) // arguments to user main(argc, argv) // argc is returned via the system call return // value, which goes in a0. - p->tf->a1 = sp; + p->trapframe->a1 = sp; // Save program name for debugging. for(last=s=path; *s; s++) @@ -109,8 +109,8 @@ exec(char *path, char **argv) oldpagetable = p->pagetable; p->pagetable = pagetable; p->sz = sz; - p->tf->epc = elf.entry; // initial program counter = main - p->tf->sp = sp; // initial stack pointer + p->trapframe->epc = elf.entry; // initial program counter = main + p->trapframe->sp = sp; // initial stack pointer proc_freepagetable(oldpagetable, oldsz); return argc; // this ends up in a0, the first argument to main(argc, argv) diff --git a/kernel/memlayout.h b/kernel/memlayout.h index 8ffd538..b6e595d 100644 --- a/kernel/memlayout.h +++ b/kernel/memlayout.h @@ -62,6 +62,6 @@ // fixed-size stack // expandable heap // ... -// TRAPFRAME (p->tf, used by the trampoline) +// TRAPFRAME (p->trapframe, used by the trampoline) // TRAMPOLINE (the same page as in the kernel) #define TRAPFRAME (TRAMPOLINE - PGSIZE) diff --git a/kernel/proc.c b/kernel/proc.c index 0cb5afe..d4ff37c 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -106,7 +106,7 @@ found: p->pid = allocpid(); // Allocate a trapframe page. - if((p->tf = (struct trapframe *)kalloc()) == 0){ + if((p->trapframe = (struct trapframe *)kalloc()) == 0){ release(&p->lock); return 0; } @@ -129,9 +129,9 @@ found: static void freeproc(struct proc *p) { - if(p->tf) - kfree((void*)p->tf); - p->tf = 0; + if(p->trapframe) + kfree((void*)p->trapframe); + p->trapframe = 0; if(p->pagetable) proc_freepagetable(p->pagetable, p->sz); p->pagetable = 0; @@ -164,7 +164,7 @@ proc_pagetable(struct proc *p) // map the trapframe just below TRAMPOLINE, for trampoline.S. mappages(pagetable, TRAPFRAME, PGSIZE, - (uint64)(p->tf), PTE_R | PTE_W); + (uint64)(p->trapframe), PTE_R | PTE_W); return pagetable; } @@ -206,8 +206,8 @@ userinit(void) p->sz = PGSIZE; // prepare for the very first "return" from kernel to user. - p->tf->epc = 0; // user program counter - p->tf->sp = PGSIZE; // user stack pointer + p->trapframe->epc = 0; // user program counter + p->trapframe->sp = PGSIZE; // user stack pointer safestrcpy(p->name, "initcode", sizeof(p->name)); p->cwd = namei("/"); @@ -262,10 +262,10 @@ fork(void) np->parent = p; // copy saved user registers. - *(np->tf) = *(p->tf); + *(np->trapframe) = *(p->trapframe); // Cause fork to return 0 in the child. - np->tf->a0 = 0; + np->trapframe->a0 = 0; // increment reference counts on open file descriptors. for(i = 0; i < NOFILE; i++) diff --git a/kernel/proc.h b/kernel/proc.h index 812c769..02e7bc3 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -98,7 +98,7 @@ struct proc { uint64 kstack; // Virtual address of kernel stack uint64 sz; // Size of process memory (bytes) pagetable_t pagetable; // Page table - struct trapframe *tf; // data page for trampoline.S + struct trapframe *trapframe; // data page for trampoline.S struct context context; // swtch() here to run process struct file *ofile[NOFILE]; // Open files struct inode *cwd; // Current directory diff --git a/kernel/syscall.c b/kernel/syscall.c index 2817e44..c1b3670 100644 --- a/kernel/syscall.c +++ b/kernel/syscall.c @@ -37,17 +37,17 @@ argraw(int n) struct proc *p = myproc(); switch (n) { case 0: - return p->tf->a0; + return p->trapframe->a0; case 1: - return p->tf->a1; + return p->trapframe->a1; case 2: - return p->tf->a2; + return p->trapframe->a2; case 3: - return p->tf->a3; + return p->trapframe->a3; case 4: - return p->tf->a4; + return p->trapframe->a4; case 5: - return p->tf->a5; + return p->trapframe->a5; } panic("argraw"); return -1; @@ -135,12 +135,12 @@ syscall(void) int num; struct proc *p = myproc(); - num = p->tf->a7; + num = p->trapframe->a7; if(num > 0 && num < NELEM(syscalls) && syscalls[num]) { - p->tf->a0 = syscalls[num](); + p->trapframe->a0 = syscalls[num](); } else { printf("%d %s: unknown sys call %d\n", p->pid, p->name, num); - p->tf->a0 = -1; + p->trapframe->a0 = -1; } } diff --git a/kernel/trap.c b/kernel/trap.c index 5e11e4b..77ff868 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -48,7 +48,7 @@ usertrap(void) struct proc *p = myproc(); // save user program counter. - p->tf->epc = r_sepc(); + p->trapframe->epc = r_sepc(); if(r_scause() == 8){ // system call @@ -58,7 +58,7 @@ usertrap(void) // sepc points to the ecall instruction, // but we want to return to the next instruction. - p->tf->epc += 4; + p->trapframe->epc += 4; // an interrupt will change sstatus &c registers, // so don't enable until done with those registers. @@ -100,10 +100,10 @@ usertrapret(void) // set up trapframe values that uservec will need when // the process next re-enters the kernel. - p->tf->kernel_satp = r_satp(); // kernel page table - p->tf->kernel_sp = p->kstack + PGSIZE; // process's kernel stack - p->tf->kernel_trap = (uint64)usertrap; - p->tf->kernel_hartid = r_tp(); // hartid for cpuid() + 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; + p->trapframe->kernel_hartid = r_tp(); // hartid for cpuid() // set up the registers that trampoline.S's sret will use // to get to user space. @@ -115,7 +115,7 @@ usertrapret(void) w_sstatus(x); // set S Exception Program Counter to the saved user pc. - w_sepc(p->tf->epc); + w_sepc(p->trapframe->epc); // tell trampoline.S the user page table to switch to. uint64 satp = MAKE_SATP(p->pagetable); From 1e72d5ca087985938589ce509ace4914039860d3 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 17 Jul 2020 16:40:57 -0400 Subject: [PATCH 22/31] cpu->scheduler -> cpu->context to reduce confusion --- kernel/proc.c | 4 ++-- kernel/proc.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index d4ff37c..f7652f6 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -456,7 +456,7 @@ scheduler(void) // before jumping back to us. p->state = RUNNING; c->proc = p; - swtch(&c->scheduler, &p->context); + swtch(&c->context, &p->context); // Process is done running for now. // It should have changed its p->state before coming back. @@ -490,7 +490,7 @@ sched(void) panic("sched interruptible"); intena = mycpu()->intena; - swtch(&p->context, &mycpu()->scheduler); + swtch(&p->context, &mycpu()->context); mycpu()->intena = intena; } diff --git a/kernel/proc.h b/kernel/proc.h index 02e7bc3..c257eb7 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -21,7 +21,7 @@ struct context { // Per-CPU state. struct cpu { struct proc *proc; // The process running on this cpu, or null. - struct context scheduler; // swtch() here to enter scheduler(). + struct context context; // swtch() here to enter scheduler(). int noff; // Depth of push_off() nesting. int intena; // Were interrupts enabled before push_off()? }; From 27057bc9b467db64a3de600f27d6fa3239a04c88 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Mon, 20 Jul 2020 06:59:26 -0400 Subject: [PATCH 23/31] interrupt-driven uart output, hopefully a nice example for teaching. --- kernel/console.c | 8 ++-- kernel/defs.h | 2 +- kernel/plic.c | 2 - kernel/trap.c | 8 +++- kernel/uart.c | 98 +++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 100 insertions(+), 18 deletions(-) diff --git a/kernel/console.c b/kernel/console.c index 87a83ff..a97ef30 100644 --- a/kernel/console.c +++ b/kernel/console.c @@ -27,6 +27,8 @@ // // send one character to the uart. +// called by printf, and to echo input characters, +// but not from write(). // void consputc(int c) @@ -40,9 +42,9 @@ consputc(int c) if(c == BACKSPACE){ // if the user typed backspace, overwrite with a space. - uartputc('\b'); uartputc(' '); uartputc('\b'); + uartputc('\b', 0); uartputc(' ', 0); uartputc('\b', 0); } else { - uartputc(c); + uartputc(c, 0); } } @@ -70,7 +72,7 @@ consolewrite(int user_src, uint64 src, int n) char c; if(either_copyin(&c, user_src, src+i, 1) == -1) break; - consputc(c); + uartputc(c, 1); } release(&cons.lock); diff --git a/kernel/defs.h b/kernel/defs.h index f33f1f6..1b164a4 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -149,7 +149,7 @@ void usertrapret(void); // uart.c void uartinit(void); void uartintr(void); -void uartputc(int); +void uartputc(int, int); int uartgetc(void); // vm.c diff --git a/kernel/plic.c b/kernel/plic.c index eed8316..5acba39 100644 --- a/kernel/plic.c +++ b/kernel/plic.c @@ -33,7 +33,6 @@ int plic_claim(void) { int hart = cpuid(); - //int irq = *(uint32*)(PLIC + 0x201004); int irq = *(uint32*)PLIC_SCLAIM(hart); return irq; } @@ -43,6 +42,5 @@ void plic_complete(int irq) { int hart = cpuid(); - //*(uint32*)(PLIC + 0x201004) = irq; *(uint32*)PLIC_SCLAIM(hart) = irq; } diff --git a/kernel/trap.c b/kernel/trap.c index 77ff868..a63249e 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -91,8 +91,9 @@ usertrapret(void) { struct proc *p = myproc(); - // turn off interrupts, since we're switching - // now from kerneltrap() to usertrap(). + // we're about to switch the destination of traps from + // kerneltrap() to usertrap(), so turn off interrupts until + // we're back in user space, where usertrap() is correct. intr_off(); // send syscalls, interrupts, and exceptions to trampoline.S @@ -192,6 +193,9 @@ devintr() printf("unexpected interrupt irq=%d\n", irq); } + // the PLIC allows each device to raise at most one + // interrupt at a time; tell the PLIC the device is + // now allowed to interrupt again. if(irq) plic_complete(irq); diff --git a/kernel/uart.c b/kernel/uart.c index 3a5cdc4..4d70b42 100644 --- a/kernel/uart.c +++ b/kernel/uart.c @@ -18,7 +18,7 @@ // the UART control registers. // some have different meanings for // read vs write. -// http://byterunner.com/16550.html +// see http://byterunner.com/16550.html #define RHR 0 // receive holding register (for input bytes) #define THR 0 // transmit holding register (for output bytes) #define IER 1 // interrupt enable register @@ -30,6 +30,15 @@ #define ReadReg(reg) (*(Reg(reg))) #define WriteReg(reg, v) (*(Reg(reg)) = (v)) +// the transmit output buffer. +struct spinlock uart_tx_lock; +#define UART_TX_BUF_SIZE 32 +char uart_tx_buf[UART_TX_BUF_SIZE]; +int uart_tx_w; // write next to uart_tx_buf[uart_tx_w++] +int uart_tx_r; // read next from uart_tx_buf[uar_tx_r++] + +void uartstart(); + void uartinit(void) { @@ -52,18 +61,79 @@ uartinit(void) // reset and enable FIFOs. WriteReg(FCR, 0x07); - // enable receive interrupts. - WriteReg(IER, 0x01); + // enable transmit and receive interrupts. + WriteReg(IER, 0x02 | 0x01); + + initlock(&uart_tx_lock, "uart"); } -// write one output character to the UART. +// add a character to the output buffer and tell the +// UART to start sending if it isn't already. +// +// usually called from the top-half -- by a process +// calling write(). can also be called from a uart +// interrupt to echo a received character, or by printf +// or panic from anywhere in the kernel. +// +// the block argument controls what happens if the +// buffer is full. for write(), block is 1, and the +// process waits. for kernel printf's and echoed +// characters, block is 0, and the character is +// discarded; this is necessary since sleep() is +// not possible in interrupts. void -uartputc(int c) +uartputc(int c, int block) { - // wait for Transmit Holding Empty to be set in LSR. - while((ReadReg(LSR) & (1 << 5)) == 0) - ; - WriteReg(THR, c); + acquire(&uart_tx_lock); + while(1){ + if(((uart_tx_w + 1) % UART_TX_BUF_SIZE) == uart_tx_r){ + // buffer is full. + if(block){ + // wait for uartstart() to open up space in the buffer. + sleep(&uart_tx_r, &uart_tx_lock); + } else { + // caller does not want us to wait. + release(&uart_tx_lock); + return; + } + } else { + uart_tx_buf[uart_tx_w] = c; + uart_tx_w = (uart_tx_w + 1) % UART_TX_BUF_SIZE; + uartstart(); + release(&uart_tx_lock); + return; + } + } +} + +// if the UART is idle, and a character is waiting +// in the transmit buffer, send it. +// caller must hold uart_tx_lock. +// called from both the top- and bottom-half. +void +uartstart() +{ + while(1){ + if(uart_tx_w == uart_tx_r){ + // transmit buffer is empty. + return; + } + + if((ReadReg(LSR) & (1 << 5)) == 0){ + // the UART transmit holding register is full, + // so we cannot give it another byte. + // it will interrupt when it's ready for a new byte. + return; + } + + int c = uart_tx_buf[uart_tx_r]; + uart_tx_r = (uart_tx_r + 1) % UART_TX_BUF_SIZE; + + // maybe uartputc() is waiting for space in the buffer. + wakeup(&uart_tx_r); + + WriteReg(THR, c); + } } // read one input character from the UART. @@ -79,14 +149,22 @@ uartgetc(void) } } -// trap.c calls here when the uart interrupts. +// handle a uart interrupt, raised because input has +// arrived, or the uart is ready for more output, or +// both. called from trap.c. void uartintr(void) { + // read and process incoming characters. while(1){ int c = uartgetc(); if(c == -1) break; consoleintr(c); } + + // send buffered characters. + acquire(&uart_tx_lock); + uartstart(); + release(&uart_tx_lock); } From 0f50e9527c68168c8114a38b211a8744e3bd2473 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Wed, 22 Jul 2020 10:31:46 -0400 Subject: [PATCH 24/31] fix printf() in interrupts --- kernel/console.c | 6 +++--- kernel/defs.h | 3 ++- kernel/uart.c | 45 ++++++++++++++++++++++++--------------------- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/kernel/console.c b/kernel/console.c index a97ef30..9a18cd9 100644 --- a/kernel/console.c +++ b/kernel/console.c @@ -42,9 +42,9 @@ consputc(int c) if(c == BACKSPACE){ // if the user typed backspace, overwrite with a space. - uartputc('\b', 0); uartputc(' ', 0); uartputc('\b', 0); + uartputc_sync('\b'); uartputc_sync(' '); uartputc_sync('\b'); } else { - uartputc(c, 0); + uartputc_sync(c); } } @@ -72,7 +72,7 @@ consolewrite(int user_src, uint64 src, int n) char c; if(either_copyin(&c, user_src, src+i, 1) == -1) break; - uartputc(c, 1); + uartputc(c); } release(&cons.lock); diff --git a/kernel/defs.h b/kernel/defs.h index 1b164a4..4eedd89 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -149,7 +149,8 @@ void usertrapret(void); // uart.c void uartinit(void); void uartintr(void); -void uartputc(int, int); +void uartputc(int); +void uartputc_sync(int); int uartgetc(void); // vm.c diff --git a/kernel/uart.c b/kernel/uart.c index 4d70b42..32cb575 100644 --- a/kernel/uart.c +++ b/kernel/uart.c @@ -69,33 +69,19 @@ uartinit(void) // add a character to the output buffer and tell the // UART to start sending if it isn't already. -// -// usually called from the top-half -- by a process -// calling write(). can also be called from a uart -// interrupt to echo a received character, or by printf -// or panic from anywhere in the kernel. -// -// the block argument controls what happens if the -// buffer is full. for write(), block is 1, and the -// process waits. for kernel printf's and echoed -// characters, block is 0, and the character is -// discarded; this is necessary since sleep() is -// not possible in interrupts. +// blocks if the output buffer is full. +// because it may block, it can't be called +// from interrupts; it's only suitable for use +// by write(). void -uartputc(int c, int block) +uartputc(int c) { acquire(&uart_tx_lock); while(1){ if(((uart_tx_w + 1) % UART_TX_BUF_SIZE) == uart_tx_r){ // buffer is full. - if(block){ - // wait for uartstart() to open up space in the buffer. - sleep(&uart_tx_r, &uart_tx_lock); - } else { - // caller does not want us to wait. - release(&uart_tx_lock); - return; - } + // wait for uartstart() to open up space in the buffer. + sleep(&uart_tx_r, &uart_tx_lock); } else { uart_tx_buf[uart_tx_w] = c; uart_tx_w = (uart_tx_w + 1) % UART_TX_BUF_SIZE; @@ -106,6 +92,23 @@ uartputc(int c, int block) } } +// 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 +// output register to be empty. +void +uartputc_sync(int c) +{ + push_off(); + + // wait for Transmit Holding Empty to be set in LSR. + while((ReadReg(LSR) & (1 << 5)) == 0) + ; + WriteReg(THR, c); + + pop_off(); +} + // if the UART is idle, and a character is waiting // in the transmit buffer, send it. // caller must hold uart_tx_lock. From 2ae9c8e2722282d9a619c1745d845fb5d862c518 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Thu, 23 Jul 2020 06:27:20 -0400 Subject: [PATCH 25/31] defines for UART register bits --- kernel/uart.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/kernel/uart.c b/kernel/uart.c index 32cb575..daf9f04 100644 --- a/kernel/uart.c +++ b/kernel/uart.c @@ -19,13 +19,21 @@ // some have different meanings for // read vs write. // see http://byterunner.com/16550.html -#define RHR 0 // receive holding register (for input bytes) -#define THR 0 // transmit holding register (for output bytes) -#define IER 1 // interrupt enable register -#define FCR 2 // FIFO control register -#define ISR 2 // interrupt status register -#define LCR 3 // line control register -#define LSR 5 // line status register +#define RHR 0 // receive holding register (for input bytes) +#define THR 0 // transmit holding register (for output bytes) +#define IER 1 // interrupt enable register +#define IER_TX_ENABLE (1<<0) +#define IER_RX_ENABLE (1<<1) +#define FCR 2 // FIFO control register +#define FCR_FIFO_ENABLE (1<<0) +#define FCR_FIFO_CLEAR (3<<1) // clear the content of the two FIFOs +#define ISR 2 // interrupt status register +#define LCR 3 // line control register +#define LCR_EIGHT_BITS (3<<0) +#define LCR_BAUD_LATCH (1<<7) // special mode to set baud rate +#define LSR 5 // line status register +#define LSR_RX_READY (1<<0) // input is waiting to be read from RHR +#define LSR_TX_IDLE (1<<5) // THR can accept another character to send #define ReadReg(reg) (*(Reg(reg))) #define WriteReg(reg, v) (*(Reg(reg)) = (v)) @@ -46,7 +54,7 @@ uartinit(void) WriteReg(IER, 0x00); // special mode to set baud rate. - WriteReg(LCR, 0x80); + WriteReg(LCR, LCR_BAUD_LATCH); // LSB for baud rate of 38.4K. WriteReg(0, 0x03); @@ -56,13 +64,13 @@ uartinit(void) // leave set-baud mode, // and set word length to 8 bits, no parity. - WriteReg(LCR, 0x03); + WriteReg(LCR, LCR_EIGHT_BITS); // reset and enable FIFOs. - WriteReg(FCR, 0x07); + WriteReg(FCR, FCR_FIFO_ENABLE | FCR_FIFO_CLEAR); // enable transmit and receive interrupts. - WriteReg(IER, 0x02 | 0x01); + WriteReg(IER, IER_TX_ENABLE | IER_RX_ENABLE); initlock(&uart_tx_lock, "uart"); } @@ -102,7 +110,7 @@ uartputc_sync(int c) push_off(); // wait for Transmit Holding Empty to be set in LSR. - while((ReadReg(LSR) & (1 << 5)) == 0) + while((ReadReg(LSR) & LSR_TX_IDLE) == 0) ; WriteReg(THR, c); @@ -122,7 +130,7 @@ uartstart() return; } - if((ReadReg(LSR) & (1 << 5)) == 0){ + if((ReadReg(LSR) & LSR_TX_IDLE) == 0){ // the UART transmit holding register is full, // so we cannot give it another byte. // it will interrupt when it's ready for a new byte. From 7f35d7a14efe56905d340a0e318f01c7c514320d Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 7 Aug 2020 05:32:48 -0400 Subject: [PATCH 26/31] modify each page in usertests countfree() get rid of static for walk() and freewalk() --- kernel/proc.c | 5 +++-- kernel/proc.h | 2 +- kernel/vm.c | 8 +++----- user/usertests.c | 5 ++++- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/kernel/proc.c b/kernel/proc.c index f7652f6..cf7f260 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -20,6 +20,7 @@ static void wakeup1(struct proc *chan); extern char trampoline[]; // trampoline.S +// initialize the proc table at boot time. void procinit(void) { @@ -145,8 +146,8 @@ freeproc(struct proc *p) p->state = UNUSED; } -// Create a page table for a given process, -// with no user pages, but with trampoline pages. +// Create a user page table for a given process, +// with no user memory, but with trampoline pages. pagetable_t proc_pagetable(struct proc *p) { diff --git a/kernel/proc.h b/kernel/proc.h index c257eb7..9c16ea7 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -97,7 +97,7 @@ struct proc { // these are private to the process, so p->lock need not be held. uint64 kstack; // Virtual address of kernel stack uint64 sz; // Size of process memory (bytes) - pagetable_t pagetable; // Page table + pagetable_t pagetable; // User page table struct trapframe *trapframe; // data page for trampoline.S struct context context; // swtch() here to run process struct file *ofile[NOFILE]; // Open files diff --git a/kernel/vm.c b/kernel/vm.c index 3004bb3..4f65d4e 100644 --- a/kernel/vm.c +++ b/kernel/vm.c @@ -16,9 +16,7 @@ extern char etext[]; // kernel.ld sets this to end of kernel code. extern char trampoline[]; // trampoline.S /* - * create a direct-map page table for the kernel and - * turn on paging. called early, in supervisor mode. - * the page allocator is already initialized. + * create a direct-map page table for the kernel. */ void kvminit() @@ -70,7 +68,7 @@ kvminithart() // 21..39 -- 9 bits of level-1 index. // 12..20 -- 9 bits of level-0 index. // 0..12 -- 12 bits of byte offset within the page. -static pte_t * +pte_t * walk(pagetable_t pagetable, uint64 va, int alloc) { if(va >= MAXVA) @@ -278,7 +276,7 @@ uvmdealloc(pagetable_t pagetable, uint64 oldsz, uint64 newsz) // Recursively free page-table pages. // All leaf mappings must already have been removed. -static void +void freewalk(pagetable_t pagetable) { // there are 2^9 = 512 PTEs in a page table. diff --git a/user/usertests.c b/user/usertests.c index f83a060..aefbc9f 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -2251,9 +2251,12 @@ countfree() int n = 0; while(1){ - if((uint64)sbrk(4096) == 0xffffffffffffffff){ + uint64 a = (uint64) sbrk(4096); + if(a == 0xffffffffffffffff){ break; } + // modify the memory to make sure it's really allocated. + *(char *)(a - 1) = 1; n += 1; } sbrk(-((uint64)sbrk(0) - sz0)); From 1f555198d61d1c447e874ae7e5a0868513822023 Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Thu, 6 Aug 2020 20:30:43 -0400 Subject: [PATCH 27/31] Change tf -> trapframe in a few comments --- kernel/trampoline.S | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/trampoline.S b/kernel/trampoline.S index b113bf6..fabaaf9 100644 --- a/kernel/trampoline.S +++ b/kernel/trampoline.S @@ -20,7 +20,7 @@ uservec: # in supervisor mode, but with a # user page table. # - # sscratch points to where the process's p->tf is + # sscratch points to where the process's p->trapframe is # mapped into user space, at TRAPFRAME. # @@ -60,20 +60,20 @@ uservec: sd t5, 272(a0) sd t6, 280(a0) - # save the user a0 in p->tf->a0 + # save the user a0 in p->trapframe->a0 csrr t0, sscratch sd t0, 112(a0) - # restore kernel stack pointer from p->tf->kernel_sp + # restore kernel stack pointer from p->trapframe->kernel_sp ld sp, 8(a0) - # make tp hold the current hartid, from p->tf->kernel_hartid + # make tp hold the current hartid, from p->trapframe->kernel_hartid ld tp, 32(a0) - # load the address of usertrap(), p->tf->kernel_trap + # load the address of usertrap(), p->trapframe->kernel_trap ld t0, 16(a0) - # restore kernel page table from p->tf->kernel_satp + # restore kernel page table from p->trapframe->kernel_satp ld t1, 0(a0) csrw satp, t1 sfence.vma zero, zero From a93321cb2547dbb48bf8ce9ad623ac19eefbecea Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 7 Aug 2020 14:34:39 -0400 Subject: [PATCH 28/31] test pointer checking in copyin, copyout, copyinstr --- user/usertests.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/user/usertests.c b/user/usertests.c index aefbc9f..bdf6970 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -22,6 +22,92 @@ char buf[BUFSZ]; char name[3]; +void +copyin1(char *s) +{ + int fd = open("copyin1", O_CREATE|O_WRONLY); + if(fd < 0){ + printf("open(copyin1) failed\n"); + exit(1); + } + int n = write(fd, (void*)0x80000000LL, 8192); + if(n >= 0){ + printf("write(fd, 0x80000000LL, 8192) did not fail!\n"); + exit(1); + } + close(fd); + unlink("copyin1"); +} + +void +copyin2(char *s) +{ + int fd = open("copyin2", O_CREATE|O_WRONLY); + if(fd < 0){ + printf("open(copyin2) failed\n"); + exit(1); + } + int n = write(fd, (void*)0xffffffffffffffffLL, 8192); + if(n >= 0){ + printf("write(fd, 0xffffffffffffffffLL, 8192) did not fail!\n"); + exit(1); + } + close(fd); + unlink("copyin2"); +} + +void +copyout1(char *s) +{ + int fd = open("README", 0); + if(fd < 0){ + printf("open(README) failed\n"); + exit(1); + } + int n = read(fd, (void*)0x80000000LL, 8192); + if(n >= 0){ + printf("read(fd, 0x80000000LL, 8192) returned %d, not -1\n", n); + exit(1); + } + close(fd); +} + +void +copyout2(char *s) +{ + int fd = open("README", 0); + if(fd < 0){ + printf("open(README) failed\n"); + exit(1); + } + int n = read(fd, (void*)0xffffffffffffffffLL, 8192); + if(n >= 0){ + printf("read(fd, 0xffffffffffffffff, 8192) returned %d, not -1\n", n); + exit(1); + } + close(fd); +} + +void +copyinstr1(char *s) +{ + int fd = open((char *)0x80000000LL, O_CREATE|O_WRONLY); + if(fd >= 0){ + printf("open(0x80000000) returned %d, not -1\n", fd); + exit(1); + } +} + +void +copyinstr2(char *s) +{ + int fd = open((char *)0xffffffffffffffff, O_CREATE|O_WRONLY); + if(fd >= 0){ + printf("open(0xffffffffffffffff) returned %d, not -1\n", fd); + exit(1); + } +} + // test O_TRUNC. void truncate1(char *s) @@ -2307,6 +2393,12 @@ main(int argc, char *argv[]) void (*f)(char *); char *s; } tests[] = { + {copyin1, "copyin1"}, + {copyin2, "copyin2"}, + {copyout1, "copyout1"}, + {copyout2, "copyout2"}, + {copyinstr1, "copyinstr1"}, + {copyinstr2, "copyinstr2"}, {truncate1, "truncate1"}, {truncate2, "truncate2"}, {truncate3, "truncate3"}, From e3b7058907dff779cf94e23bf6bb84245faf481d Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 7 Aug 2020 15:06:43 -0400 Subject: [PATCH 29/31] streamline copyin/copyout code in usertests fix bugs in read/write return values when there's an error --- kernel/console.c | 2 +- kernel/fs.c | 2 +- kernel/pipe.c | 2 +- user/usertests.c | 171 +++++++++++++++++++++++++---------------------- 4 files changed, 93 insertions(+), 84 deletions(-) diff --git a/kernel/console.c b/kernel/console.c index 9a18cd9..2b1ed3c 100644 --- a/kernel/console.c +++ b/kernel/console.c @@ -76,7 +76,7 @@ consolewrite(int user_src, uint64 src, int n) } release(&cons.lock); - return n; + return i; } // diff --git a/kernel/fs.c b/kernel/fs.c index e33ec30..ec68cd7 100644 --- a/kernel/fs.c +++ b/kernel/fs.c @@ -472,7 +472,7 @@ readi(struct inode *ip, int user_dst, uint64 dst, uint off, uint n) } brelse(bp); } - return n; + return tot; } // Write data to inode. diff --git a/kernel/pipe.c b/kernel/pipe.c index c066afb..7ed402d 100644 --- a/kernel/pipe.c +++ b/kernel/pipe.c @@ -96,7 +96,7 @@ pipewrite(struct pipe *pi, uint64 addr, int n) } wakeup(&pi->nread); release(&pi->lock); - return n; + return i; } int diff --git a/user/usertests.c b/user/usertests.c index bdf6970..8eb4aab 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -23,88 +23,100 @@ char buf[BUFSZ]; char name[3]; void -copyin1(char *s) +copyin(char *s) { - int fd = open("copyin1", O_CREATE|O_WRONLY); - if(fd < 0){ - printf("open(copyin1) failed\n"); - exit(1); - } - int n = write(fd, (void*)0x80000000LL, 8192); - if(n >= 0){ - printf("write(fd, 0x80000000LL, 8192) did not fail!\n"); - exit(1); - } - close(fd); - unlink("copyin1"); -} + uint64 addrs[] = { 0x80000000LL, 0xffffffffffffffff }; -void -copyin2(char *s) -{ - int fd = open("copyin2", O_CREATE|O_WRONLY); - if(fd < 0){ - printf("open(copyin2) failed\n"); - exit(1); - } - int n = write(fd, (void*)0xffffffffffffffffLL, 8192); - if(n >= 0){ - printf("write(fd, 0xffffffffffffffffLL, 8192) did not fail!\n"); - exit(1); - } - close(fd); - unlink("copyin2"); -} - -void -copyout1(char *s) -{ - int fd = open("README", 0); - if(fd < 0){ - printf("open(README) failed\n"); - exit(1); - } - int n = read(fd, (void*)0x80000000LL, 8192); - if(n >= 0){ - printf("read(fd, 0x80000000LL, 8192) returned %d, not -1\n", n); - exit(1); - } - close(fd); -} - -void -copyout2(char *s) -{ - int fd = open("README", 0); - if(fd < 0){ - printf("open(README) failed\n"); - exit(1); - } - int n = read(fd, (void*)0xffffffffffffffffLL, 8192); - if(n >= 0){ - printf("read(fd, 0xffffffffffffffff, 8192) returned %d, not -1\n", n); - exit(1); - } - close(fd); -} - -void -copyinstr1(char *s) -{ - int fd = open((char *)0x80000000LL, O_CREATE|O_WRONLY); - if(fd >= 0){ - printf("open(0x80000000) returned %d, not -1\n", fd); - exit(1); + for(int ai = 0; ai < 2; ai++){ + uint64 addr = addrs[ai]; + + int fd = open("copyin1", O_CREATE|O_WRONLY); + if(fd < 0){ + printf("open(copyin1) failed\n"); + exit(1); + } + int n = write(fd, (void*)addr, 8192); + if(n >= 0){ + printf("write(fd, %p, 8192) returned %d, not -1\n", addr, n); + exit(1); + } + close(fd); + unlink("copyin1"); + + n = write(1, (char*)addr, 8192); + if(n > 0){ + printf("write(1, %p, 8192) returned %d, not -1 or 0\n", addr, n); + exit(1); + } + + int fds[2]; + if(pipe(fds) < 0){ + printf("pipe() failed\n"); + exit(1); + } + n = write(fds[1], (char*)addr, 8192); + if(n > 0){ + printf("write(pipe, %p, 8192) returned %d, not -1 or 0\n", addr, n); + exit(1); + } + close(fds[0]); + close(fds[1]); } } void -copyinstr2(char *s) +copyout(char *s) { - int fd = open((char *)0xffffffffffffffff, O_CREATE|O_WRONLY); - if(fd >= 0){ - printf("open(0xffffffffffffffff) returned %d, not -1\n", fd); - exit(1); + uint64 addrs[] = { 0x80000000LL, 0xffffffffffffffff }; + + for(int ai = 0; ai < 2; ai++){ + uint64 addr = addrs[ai]; + + int fd = open("README", 0); + if(fd < 0){ + printf("open(README) failed\n"); + exit(1); + } + int n = read(fd, (void*)addr, 8192); + if(n > 0){ + printf("read(fd, %p, 8192) returned %d, not -1 or 0\n", addr, n); + exit(1); + } + close(fd); + + int fds[2]; + if(pipe(fds) < 0){ + printf("pipe() failed\n"); + exit(1); + } + n = write(fds[1], "x", 1); + if(n != 1){ + printf("pipe write failed\n"); + exit(1); + } + n = read(fds[0], (void*)addr, 8192); + if(n > 0){ + printf("read(pipe, %p, 8192) returned %d, not -1 or 0\n", addr, n); + exit(1); + } + close(fds[0]); + close(fds[1]); + } +} + +void +copyinstr(char *s) +{ + uint64 addrs[] = { 0x80000000LL, 0xffffffffffffffff }; + + for(int ai = 0; ai < 2; ai++){ + uint64 addr = addrs[ai]; + + int fd = open((char *)addr, O_CREATE|O_WRONLY); + if(fd >= 0){ + printf("open(%p) returned %d, not -1\n", addr, fd); + exit(1); + } } } @@ -2393,12 +2405,9 @@ main(int argc, char *argv[]) void (*f)(char *); char *s; } tests[] = { - {copyin1, "copyin1"}, - {copyin2, "copyin2"}, - {copyout1, "copyout1"}, - {copyout2, "copyout2"}, - {copyinstr1, "copyinstr1"}, - {copyinstr2, "copyinstr2"}, + {copyin, "copyin"}, + {copyout, "copyout"}, + {copyinstr, "copyinstr"}, {truncate1, "truncate1"}, {truncate2, "truncate2"}, {truncate3, "truncate3"}, From 76d6c57edec14071156e7780f7e8e08c200cf166 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 7 Aug 2020 16:39:56 -0400 Subject: [PATCH 30/31] test copyinstr()'s handling of the terminating null --- user/usertests.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/user/usertests.c b/user/usertests.c index 8eb4aab..5c2fc02 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -105,7 +105,7 @@ copyout(char *s) } void -copyinstr(char *s) +copyinstr1(char *s) { uint64 addrs[] = { 0x80000000LL, 0xffffffffffffffff }; @@ -120,6 +120,67 @@ copyinstr(char *s) } } +void +copyinstr2(char *s) +{ + char b[MAXPATH+1]; + + for(int i = 0; i < MAXPATH; i++) + b[i] = 'x'; + b[MAXPATH] = '\0'; + + int ret = unlink(b); + if(ret != -1){ + printf("unlink(%s) returned %d, not -1\n", b, ret); + exit(1); + } + + int fd = open(b, O_CREATE | O_WRONLY); + if(fd != -1){ + printf("open(%s) returned %d, not -1\n", b, fd); + exit(1); + } + + ret = link(b, b); + if(ret != -1){ + printf("link(%s, %s) returned %d, not -1\n", b, b, ret); + exit(1); + } + + char *args[] = { "xx", 0 }; + ret = exec(b, args); + if(ret != -1){ + printf("exec(%s) returned %d, not -1\n", b, fd); + exit(1); + } + + int pid = fork(); + if(pid < 0){ + printf("fork failed\n"); + exit(1); + } + if(pid == 0){ + static char big[PGSIZE+1]; + for(int i = 0; i < PGSIZE; i++) + big[i] = 'x'; + big[PGSIZE] = '\0'; + char *args2[] = { big, big, big, 0 }; + ret = exec("echo", args2); + if(ret != -1){ + printf("exec(echo, BIG) returned %d, not -1\n", fd); + exit(1); + } + exit(747); // OK + } + + int st = 0; + wait(&st); + if(st != 747){ + printf("exec(echo, BIG) succeeded, should have failed\n"); + exit(1); + } +} + // test O_TRUNC. void truncate1(char *s) @@ -2407,7 +2468,8 @@ main(int argc, char *argv[]) } tests[] = { {copyin, "copyin"}, {copyout, "copyout"}, - {copyinstr, "copyinstr"}, + {copyinstr1, "copyinstr1"}, + {copyinstr2, "copyinstr2"}, {truncate1, "truncate1"}, {truncate2, "truncate2"}, {truncate3, "truncate3"}, From d8fe1773b26758c7c7b8f36724cd822555b33612 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Fri, 7 Aug 2020 16:56:00 -0400 Subject: [PATCH 31/31] test string system call arguments that cross over the end of the last page. --- user/usertests.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/user/usertests.c b/user/usertests.c index 5c2fc02..dfe0039 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -22,6 +22,8 @@ char buf[BUFSZ]; char name[3]; +// what if you pass ridiculous pointers to system calls +// that read user memory with copyin? void copyin(char *s) { @@ -64,6 +66,8 @@ copyin(char *s) } } +// what if you pass ridiculous pointers to system calls +// that write user memory with copyout? void copyout(char *s) { @@ -104,6 +108,7 @@ copyout(char *s) } } +// what if you pass ridiculous string pointers to system calls? void copyinstr1(char *s) { @@ -120,6 +125,9 @@ copyinstr1(char *s) } } +// what if a string system call argument is exactly the size +// of the kernel buffer it is copied into, so that the null +// would fall just beyond the end of the kernel buffer? void copyinstr2(char *s) { @@ -181,6 +189,50 @@ copyinstr2(char *s) } } +// what if a string argument crosses over the end of last user page? +void +copyinstr3(char *s) +{ + sbrk(8192); + uint64 top = (uint64) sbrk(0); + if((top % PGSIZE) != 0){ + sbrk(PGSIZE - (top % PGSIZE)); + } + top = (uint64) sbrk(0); + if(top % PGSIZE){ + printf("oops\n"); + exit(1); + } + + char *b = (char *) (top - 1); + *b = 'x'; + + int ret = unlink(b); + if(ret != -1){ + printf("unlink(%s) returned %d, not -1\n", b, ret); + exit(1); + } + + int fd = open(b, O_CREATE | O_WRONLY); + if(fd != -1){ + printf("open(%s) returned %d, not -1\n", b, fd); + exit(1); + } + + ret = link(b, b); + if(ret != -1){ + printf("link(%s, %s) returned %d, not -1\n", b, b, ret); + exit(1); + } + + char *args[] = { "xx", 0 }; + ret = exec(b, args); + if(ret != -1){ + printf("exec(%s) returned %d, not -1\n", b, fd); + exit(1); + } +} + // test O_TRUNC. void truncate1(char *s) @@ -2470,6 +2522,7 @@ main(int argc, char *argv[]) {copyout, "copyout"}, {copyinstr1, "copyinstr1"}, {copyinstr2, "copyinstr2"}, + {copyinstr3, "copyinstr3"}, {truncate1, "truncate1"}, {truncate2, "truncate2"}, {truncate3, "truncate3"},