summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrtm <rtm>2006-08-08 19:58:06 +0000
committerrtm <rtm>2006-08-08 19:58:06 +0000
commit0e84a0ec6e7893dad13dff9a958c5bc987b79c82 (patch)
tree5739d0a2af8277db7a47c74e52975d9e9d81cef7
parente8d11c2e846ad15b32caacc8a919722b76d00f79 (diff)
downloadxv6-labs-0e84a0ec6e7893dad13dff9a958c5bc987b79c82.tar.gz
xv6-labs-0e84a0ec6e7893dad13dff9a958c5bc987b79c82.tar.bz2
xv6-labs-0e84a0ec6e7893dad13dff9a958c5bc987b79c82.zip
fix race in holding() check in acquire()
give cpu1 a TSS and gdt for when it enters scheduler() and a pseudo proc[] entry for each cpu cpu0 waits for each other cpu to start up read() for files
-rw-r--r--Makefile8
-rw-r--r--Notes78
-rw-r--r--README1
-rw-r--r--cat.c35
-rw-r--r--echo.c2
-rw-r--r--fd.c7
-rw-r--r--fd.h1
-rw-r--r--ide.c4
-rw-r--r--ioapic.c6
-rw-r--r--lapic.c11
-rw-r--r--main.c35
-rw-r--r--mp.c4
-rw-r--r--proc.c10
-rw-r--r--proc.h7
-rw-r--r--spinlock.c28
-rw-r--r--spinlock.h2
-rw-r--r--syscall.c1
-rw-r--r--trap.c16
-rw-r--r--user.h2
-rw-r--r--userfs.c6
20 files changed, 209 insertions, 55 deletions
diff --git a/Makefile b/Makefile
index c0153ad..cc83f3e 100644
--- a/Makefile
+++ b/Makefile
@@ -71,6 +71,10 @@ echo : echo.o $(ULIB)
$(LD) -N -e main -Ttext 0 -o echo echo.o $(ULIB)
$(OBJDUMP) -S echo > echo.asm
+cat : cat.o $(ULIB)
+ $(LD) -N -e main -Ttext 0 -o cat cat.o $(ULIB)
+ $(OBJDUMP) -S cat > cat.asm
+
userfs : userfs.o $(ULIB)
$(LD) -N -e main -Ttext 0 -o userfs userfs.o $(ULIB)
$(OBJDUMP) -S userfs > userfs.asm
@@ -78,8 +82,8 @@ userfs : userfs.o $(ULIB)
mkfs : mkfs.c fs.h
cc -o mkfs mkfs.c
-fs.img : mkfs usertests echo
- ./mkfs fs.img usertests echo
+fs.img : mkfs usertests echo cat README
+ ./mkfs fs.img usertests echo cat README
-include *.d
diff --git a/Notes b/Notes
index 22658fc..2291621 100644
--- a/Notes
+++ b/Notes
@@ -163,3 +163,81 @@ and file arguments longer than 14
and directories longer than one sector
kalloc() can return 0; do callers handle this right?
+
+why directing interrupts to cpu 1 causes trouble
+ cpu 1 turns on interrupts with no tss!
+ and perhaps a stale gdt (from boot)
+ since it has never run a process, never called setupsegs()
+ but does cpu really need the tss?
+ not switching stacks
+ fake process per cpu, just for tss?
+ seems like a waste
+ move tss to cpu[]?
+ but tss points to per-process kernel stack
+ would also give us a gdt
+ OOPS that wasn't the problem
+
+wait for other cpu to finish starting before enabling interrupts?
+ some kind of crash in ide_init ioapic_enable cprintf
+move ide_init before mp_start?
+ didn't do any good
+ maybe cpu0 taking ide interrupt, cpu1 getting a nested lock error
+
+cprintfs are screwed up if locking is off
+ often loops forever
+ hah, just use lpt alone
+
+looks like cpu0 took the ide interrupt and was the last to hold
+the lock, but cpu1 thinks it is nested
+cpu0 is in load_icode / printf / cons_putc
+ probably b/c cpu1 cleared use_console_lock
+cpu1 is in scheduler() / printf / acquire
+
+ 1: init timer
+ 0: init timer
+ cpu 1 initial nlock 1
+ ne0s:t iidd el_occnkt rc
+ onsole cpu 1 old caller stack 1001A5 10071D 104DFF 1049FE
+ panic: acquire
+ ^CNext at t=33002418
+ (0) [0x00100091] 0008:0x00100091 (unk. ctxt): jmp .+0xfffffffe ; ebfe
+ (1) [0x00100332] 0008:0x00100332 (unk. ctxt): jmp .+0xfffffffe
+
+why is output interleaved even before panic?
+
+does release turn on interrupts even inside an interrupt handler?
+
+overflowing cpu[] stack?
+ probably not, change from 512 to 4096 didn't do anything
+
+
+ 1: init timer
+ 0: init timer
+ cnpeus te11 linnitki aclo nnoolleek cp1u
+ ss oarltd sccahleldeul esrt aocnk cpu 0111 Ej6 buf1 01A3140 C5118
+ 0
+ la anic1::7 0a0c0 uuirr e
+ ^CNext at t=31691050
+ (0) [0x00100373] 0008:0x00100373 (unk. ctxt): jmp .+0xfffffffe ; ebfe
+ (1) [0x00100091] 0008:0x00100091 (unk. ctxt): jmp .+0xfffffffe ; ebfe
+
+cpu0:
+
+0: init timer
+nested lock console cpu 0 old caller stack 1001e6 101a34 1 0
+ (that's mpmain)
+panic: acquire
+
+cpu1:
+
+1: init timer
+cpu 1 initial nlock 1
+start scheduler on cpu 1 jmpbuf ...
+la 107000 lr ...
+ that is, nlock != 0
+
+maybe a race; acquire does
+ locked = 1
+ cpu = cpu()
+what if another acquire calls holding w/ locked = 1 but
+ before cpu is set?
diff --git a/README b/README
new file mode 100644
index 0000000..ce0bbfb
--- /dev/null
+++ b/README
@@ -0,0 +1 @@
+This is the content of file README.
diff --git a/cat.c b/cat.c
new file mode 100644
index 0000000..8154ae2
--- /dev/null
+++ b/cat.c
@@ -0,0 +1,35 @@
+#include "user.h"
+
+char buf[513];
+
+int
+main(int argc, char *argv[])
+{
+ int fd, i, cc;
+
+ if(argc < 2){
+ puts("Usage: cat files...\n");
+ exit();
+ }
+
+ for(i = 1; i < argc; i++){
+ fd = open(argv[i], 0);
+ if(fd < 0){
+ puts("cat: cannot open ");
+ puts(argv[i]);
+ puts("\n");
+ exit();
+ }
+ while((cc = read(fd, buf, sizeof(buf) - 1)) > 0){
+ buf[cc] = '\0';
+ puts(buf);
+ }
+ if(cc < 0){
+ puts("cat: read error\n");
+ exit();
+ }
+ close(fd);
+ }
+
+ exit();
+}
diff --git a/echo.c b/echo.c
index 89e19a9..5d0c5a4 100644
--- a/echo.c
+++ b/echo.c
@@ -5,7 +5,7 @@ main(int argc, char *argv[])
{
int i;
- for(i = 0; i < argc; i++){
+ for(i = 1; i < argc; i++){
puts(argv[i]);
puts(" ");
}
diff --git a/fd.c b/fd.c
index 9ce6bae..8332454 100644
--- a/fd.c
+++ b/fd.c
@@ -69,6 +69,13 @@ fd_read(struct fd *fd, char *addr, int n)
return -1;
if(fd->type == FD_PIPE){
return pipe_read(fd->pipe, addr, n);
+ } else if(fd->type == FD_FILE){
+ ilock(fd->ip);
+ int cc = readi(fd->ip, addr, fd->off, n);
+ if(cc > 0)
+ fd->off += cc;
+ iunlock(fd->ip);
+ return cc;
} else {
panic("fd_read");
return -1;
diff --git a/fd.h b/fd.h
index 442ee34..1be1cf0 100644
--- a/fd.h
+++ b/fd.h
@@ -5,6 +5,7 @@ struct fd {
char writeable;
struct pipe *pipe;
struct inode *ip;
+ uint off;
};
extern struct fd fds[NFD];
diff --git a/ide.c b/ide.c
index d6bef6d..af509fc 100644
--- a/ide.c
+++ b/ide.c
@@ -51,14 +51,14 @@ ide_init(void)
}
ioapic_enable (14, 1); // 14 is IRQ # for IDE
ide_wait_ready(0);
- cprintf ("ide_init:done\n");
+ cprintf ("cpu%d: ide_init:done\n", cpu());
}
void
ide_intr(void)
{
acquire(&ide_lock);
- cprintf("%d: ide_intr\n", cpu());
+ cprintf("cpu%d: ide_intr\n", cpu());
wakeup(&request[tail]);
release(&ide_lock);
lapic_eoi();
diff --git a/ioapic.c b/ioapic.c
index 776f895..b926863 100644
--- a/ioapic.c
+++ b/ioapic.c
@@ -65,7 +65,7 @@ ioapic_init(void)
}
void
-ioapic_enable (int irq, int cpu)
+ioapic_enable (int irq, int cpunum)
{
uint l, h;
struct ioapic *io;
@@ -76,7 +76,7 @@ ioapic_enable (int irq, int cpu)
ioapic_write(io, IOAPIC_REDTBL_LO(irq), l);
h = ioapic_read(io, IOAPIC_REDTBL_HI(irq));
h &= ~IOART_DEST;
- h |= (cpu << APIC_ID_SHIFT); // for fun, disk interrupts to cpu 1
+ h |= (cpunum << APIC_ID_SHIFT); // for fun, disk interrupts to cpu 1
ioapic_write(io, IOAPIC_REDTBL_HI(irq), h);
- cprintf("intr %d: lo 0x%x hi 0x%x\n", irq, l, h);
+ cprintf("cpu%d: intr %d: lo 0x%x hi 0x%x\n", cpu(), irq, l, h);
}
diff --git a/lapic.c b/lapic.c
index f299b86..161d0a5 100644
--- a/lapic.c
+++ b/lapic.c
@@ -110,7 +110,7 @@ lapic_write(int r, int data)
void
lapic_timerinit(void)
{
- cprintf("%d: init timer\n", cpu());
+ cprintf("cpu%d: init timer\n", cpu());
lapic_write(LAPIC_TDCR, LAPIC_X1);
lapic_write(LAPIC_TIMER, LAPIC_CLKIN | LAPIC_PERIODIC | (IRQ_OFFSET + IRQ_TIMER));
lapic_write(LAPIC_TCCR, 10000000);
@@ -120,7 +120,7 @@ lapic_timerinit(void)
void
lapic_timerintr(void)
{
- cprintf("%d: timer interrupt!\n", cpu());
+ cprintf("cpu%d: timer interrupt!\n", cpu());
lapic_write (LAPIC_EOI, 0);
}
@@ -129,7 +129,7 @@ lapic_init(int c)
{
uint r, lvt;
- cprintf("lapic_init %d\n", c);
+ cprintf("cpu%d: lapic_init %d\n", c);
lapic_write(LAPIC_DFR, 0xFFFFFFFF); // set destination format register
r = (lapic_read(LAPIC_ID)>>24) & 0xFF; // read APIC ID
@@ -158,7 +158,7 @@ lapic_init(int c)
while(lapic_read(LAPIC_ICRLO) & APIC_DELIVS)
;
- cprintf("Done init of an apic\n");
+ cprintf("cpu%d: apic init done\n", cpu());
}
void
@@ -182,7 +182,8 @@ lapic_eoi(void)
int
cpu(void)
{
- return (lapic_read(LAPIC_ID)>>24) & 0xFF;
+ int x = (lapic_read(LAPIC_ID)>>24) & 0xFF;
+ return x;
}
void
diff --git a/main.c b/main.c
index 021fb51..c6e27a5 100644
--- a/main.c
+++ b/main.c
@@ -42,18 +42,24 @@ main0(void)
lapic_init(mp_bcpu());
- cprintf("\nxV6\n\n");
+ cprintf("\n\ncpu%d: booting xv6\n\n", cpu());
pic_init(); // initialize PIC
ioapic_init();
kinit(); // physical memory allocator
tvinit(); // trap vectors
- idtinit(); // CPU's idt
+ idtinit(); // this CPU's idt register
+
+ // create a fake process per CPU
+ // so each CPU always has a tss and a gdt
+ for(p = &proc[0]; p < &proc[NCPU]; p++){
+ p->state = IDLEPROC;
+ p->kstack = cpus[p-proc].mpstack;
+ p->pid = p - proc;
+ }
- // create fake process zero
+ // fix process 0 so that copyproc() will work
p = &proc[0];
- memset(p, 0, sizeof *p);
- p->state = SLEEPING;
p->sz = 4 * PAGE;
p->mem = kalloc(p->sz);
memset(p->mem, 0, p->sz);
@@ -63,20 +69,20 @@ main0(void)
p->tf->es = p->tf->ds = p->tf->ss = (SEG_UDATA << 3) | 3;
p->tf->cs = (SEG_UCODE << 3) | 3;
p->tf->eflags = FL_IF;
- p->pid = 0;
- p->ppid = 0;
setupsegs(p);
+ // init disk device
+ ide_init();
+
mp_startthem();
// turn on timer and enable interrupts on the local APIC
lapic_timerinit();
lapic_enableintr();
- // init disk device
- ide_init();
-
// Enable interrupts on this processor.
+ cprintf("cpu%d: nlock %d before -- and sti\n",
+ cpu(), cpus[0].nlock);
cpus[cpu()].nlock--;
sti();
@@ -94,7 +100,7 @@ main0(void)
void
mpmain(void)
{
- cprintf("an application processor\n");
+ cprintf("cpu%d: starting\n", cpu());
idtinit(); // CPU's idt
if(cpu() == 0)
panic("mpmain on cpu 0");
@@ -102,8 +108,13 @@ mpmain(void)
lapic_timerinit();
lapic_enableintr();
+ setupsegs(&proc[cpu()]);
+
+ cpuid(0, 0, 0, 0, 0); // memory barrier
+ cpus[cpu()].booted = 1;
+
// Enable interrupts on this processor.
- cprintf("cpu %d initial nlock %d\n", cpu(), cpus[cpu()].nlock);
+ cprintf("cpu%d: initial nlock %d\n", cpu(), cpus[cpu()].nlock);
cpus[cpu()].nlock--;
sti();
diff --git a/mp.c b/mp.c
index ece46a4..341b545 100644
--- a/mp.c
+++ b/mp.c
@@ -219,9 +219,11 @@ mp_startthem(void)
for(c = 0; c < ncpu; c++){
if (c == cpu()) continue;
- cprintf ("starting processor %d\n", c);
+ cprintf ("cpu%d: starting processor %d\n", cpu(), c);
*(uint *)(APBOOTCODE-4) = (uint) (cpus[c].mpstack) + MPSTACK; // tell it what to use for %esp
*(uint *)(APBOOTCODE-8) = (uint)mpmain; // tell it where to jump to
lapic_startap(cpus[c].apicid, (uint) APBOOTCODE);
+ while(cpus[c].booted == 0)
+ ;
}
}
diff --git a/proc.c b/proc.c
index b67810e..0254c2e 100644
--- a/proc.c
+++ b/proc.c
@@ -11,7 +11,7 @@ struct spinlock proc_table_lock = { "proc_table" };
struct proc proc[NPROC];
struct proc *curproc[NCPU];
-int next_pid = 1;
+int next_pid = NCPU;
extern void forkret(void);
extern void forkret1(struct trapframe*);
@@ -31,7 +31,7 @@ setupsegs(struct proc *p)
// XXX it may be wrong to modify the current segment table!
p->gdt[0] = SEG_NULL;
- p->gdt[SEG_KCODE] = SEG(STA_X|STA_R, 0, 0xffffffff, 0);
+ p->gdt[SEG_KCODE] = SEG(STA_X|STA_R, 0, 0x100000 + 64*1024, 0); // xxx
p->gdt[SEG_KDATA] = SEG(STA_W, 0, 0xffffffff, 0);
p->gdt[SEG_TSS] = SEG16(STS_T32A, (uint) &p->ts,
sizeof(p->ts), 0);
@@ -134,8 +134,8 @@ scheduler(void)
struct proc *p;
int i;
- cprintf("start scheduler on cpu %d jmpbuf %p\n", cpu(), &cpus[cpu()].jmpbuf);
- cpus[cpu()].lastproc = &proc[0];
+ cprintf("cpu%d: start scheduler jmpbuf %p\n",
+ cpu(), &cpus[cpu()].jmpbuf);
if(cpus[cpu()].nlock != 0){
cprintf("la %x lr %x\n", cpus[cpu()].lastacquire, cpus[cpu()].lastrelease );
@@ -190,6 +190,8 @@ scheduler(void)
panic("scheduler lock");
}
+ setupsegs(&proc[cpu()]);
+
// XXX if not holding proc_table_lock panic.
}
release(&proc_table_lock);
diff --git a/proc.h b/proc.h
index a273141..c6f5be6 100644
--- a/proc.h
+++ b/proc.h
@@ -33,7 +33,8 @@ struct jmpbuf {
int eip;
};
-enum proc_state { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
+enum proc_state { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE,
+ IDLEPROC };
struct proc{
char *mem; // start of process's physical memory
@@ -67,8 +68,8 @@ extern struct proc *curproc[NCPU]; // can be NULL if no proc running.
struct cpu {
uchar apicid; // Local APIC ID
struct jmpbuf jmpbuf;
- char mpstack[MPSTACK]; // per-cpu start-up stack, only used to get into main()
- struct proc *lastproc; // last proc scheduled on this cpu (never NULL)
+ char mpstack[MPSTACK]; // per-cpu start-up stack
+ volatile int booted;
int nlock; // # of locks currently held
struct spinlock *lastacquire; // xxx debug
struct spinlock *lastrelease; // xxx debug
diff --git a/spinlock.c b/spinlock.c
index bde6e46..5a0fd23 100644
--- a/spinlock.c
+++ b/spinlock.c
@@ -12,29 +12,31 @@
extern int use_console_lock;
-int
-getcallerpc(void *v)
+void
+getcallerpcs(void *v, uint pcs[])
{
- return ((int*)v)[-1];
+ uint *ebp = (uint*)v - 2;
+ int i;
+ for(i = 0; i < 10 && ebp && ebp != (uint*)0xffffffff; ebp = (uint*)*ebp, i++){
+ pcs[i] = *(ebp + 1);
+ }
+ for( ; i < 10; i++)
+ pcs[i] = 0;
}
void
acquire(struct spinlock * lock)
{
- if(holding(lock)){
- extern use_console_lock;
- use_console_lock = 0;
- cprintf("lock %s pc %x\n", lock->name ? lock->name : "", lock->pc);
- panic("acquire");
- }
+ if(holding(lock))
+ panic("acquire");
if(cpus[cpu()].nlock++ == 0)
cli();
while(cmpxchg(0, 1, &lock->locked) == 1)
;
cpuid(0, 0, 0, 0, 0); // memory barrier
- lock->pc = getcallerpc(&lock);
- lock->cpu = cpu();
+ getcallerpcs(&lock, lock->pcs);
+ lock->cpu = cpu() + 10;
cpus[cpu()].lastacquire = lock;
}
@@ -45,6 +47,8 @@ release(struct spinlock * lock)
panic("release");
cpus[cpu()].lastrelease = lock;
+ lock->pcs[0] = 0;
+ lock->cpu = 0xffffffff;
cpuid(0, 0, 0, 0, 0); // memory barrier
lock->locked = 0;
if(--cpus[cpu()].nlock == 0)
@@ -54,5 +58,5 @@ release(struct spinlock * lock)
int
holding(struct spinlock *lock)
{
- return lock->locked && lock->cpu == cpu();
+ return lock->locked && lock->cpu == cpu() + 10;
}
diff --git a/spinlock.h b/spinlock.h
index ee48a7e..f866b4c 100644
--- a/spinlock.h
+++ b/spinlock.h
@@ -1,6 +1,6 @@
struct spinlock {
char *name;
uint locked;
- uint pc;
+ uint pcs[10];
int cpu;
};
diff --git a/syscall.c b/syscall.c
index 498078e..ce6e22d 100644
--- a/syscall.c
+++ b/syscall.c
@@ -274,6 +274,7 @@ sys_open(void)
fd->readable = 1;
fd->writeable = 0;
fd->ip = ip;
+ fd->off = 0;
cp->fds[ufd] = fd;
return ufd;
diff --git a/trap.c b/trap.c
index ccbc754..7031223 100644
--- a/trap.c
+++ b/trap.c
@@ -34,6 +34,14 @@ void
trap(struct trapframe *tf)
{
int v = tf->trapno;
+
+ if(cpus[cpu()].nlock){
+ cprintf("trap v %d eip %x cpu %d nlock %d\n",
+ v, tf->eip, cpu(), cpus[cpu()].nlock);
+ panic("interrupt while holding a lock");
+ }
+ if((read_eflags() & FL_IF) == 0)
+ panic("interrupt but interrupts now disabled");
if(v == T_SYSCALL){
struct proc *cp = curproc[cpu()];
@@ -61,16 +69,13 @@ trap(struct trapframe *tf)
panic("trap ret esp wrong");
if(cp->killed)
proc_exit();
+ // XXX probably ought to lgdt on trap return
return;
}
if(v == (IRQ_OFFSET + IRQ_TIMER)){
struct proc *cp = curproc[cpu()];
lapic_timerintr();
- if(cpus[cpu()].nlock)
- panic("timer interrupt while holding a lock");
- if((read_eflags() & FL_IF) == 0)
- panic("timer interrupt but interrupts now disabled");
if(cp){
// Force process exit if it has been killed
// and the interrupt came from user space.
@@ -92,8 +97,5 @@ trap(struct trapframe *tf)
return;
}
-
- // XXX probably ought to lgdt on trap return
-
return;
}
diff --git a/user.h b/user.h
index cf98816..d869338 100644
--- a/user.h
+++ b/user.h
@@ -10,6 +10,8 @@ int block(void);
int kill(int);
int panic(char*);
int cons_puts(char*);
+int exec(char *, char **);
+int open(char *, int);
int mknod (char*,short,short,short);
int puts(char*);
int puts1(char*);
diff --git a/userfs.c b/userfs.c
index 0b2e9c3..b11f3eb 100644
--- a/userfs.c
+++ b/userfs.c
@@ -5,7 +5,8 @@
// file system tests
char buf[1024];
-char *args[] = { "echo", "hello", "goodbye", 0 };
+char *echo_args[] = { "echo", "hello", "goodbye", 0 };
+char *cat_args[] = { "cat", "README", 0 };
int
main(void)
@@ -34,6 +35,7 @@ main(void)
} else {
puts("open doesnotexist failed\n");
}
- exec("echo", args);
+ //exec("echo", echo_args);
+ exec("cat", cat_args);
return 0;
}