Skip to content

Commit

Permalink
fix race in holding() check in acquire()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rtm committed Aug 8, 2006
1 parent e8d11c2 commit 0e84a0e
Show file tree
Hide file tree
Showing 20 changed files with 209 additions and 55 deletions.
8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,19 @@ 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

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

Expand Down
78 changes: 78 additions & 0 deletions Notes
Original file line number Diff line number Diff line change
Expand Up @@ -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?
1 change: 1 addition & 0 deletions README
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is the content of file README.
35 changes: 35 additions & 0 deletions cat.c
Original file line number Diff line number Diff line change
@@ -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();
}
2 changes: 1 addition & 1 deletion echo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(" ");
}
Expand Down
7 changes: 7 additions & 0 deletions fd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions fd.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ struct fd {
char writeable;
struct pipe *pipe;
struct inode *ip;
uint off;
};

extern struct fd fds[NFD];
4 changes: 2 additions & 2 deletions ide.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions ioapic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
11 changes: 6 additions & 5 deletions lapic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 23 additions & 12 deletions main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();

Expand All @@ -94,16 +100,21 @@ 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");
lapic_init(cpu());
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();

Expand Down
4 changes: 3 additions & 1 deletion mp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
;
}
}
10 changes: 6 additions & 4 deletions proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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*);

Expand All @@ -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);
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -190,6 +190,8 @@ scheduler(void)
panic("scheduler lock");
}

setupsegs(&proc[cpu()]);

// XXX if not holding proc_table_lock panic.
}
release(&proc_table_lock);
Expand Down
Loading

0 comments on commit 0e84a0e

Please sign in to comment.