Skip to content

Commit

Permalink
i think my cmpxchg use was wrong in acquire
Browse files Browse the repository at this point in the history
nesting cli/sti: release shouldn't always enable interrupts
separate setup of lapic from starting of other cpus, so cpu() works earlier
flag to disable locking in console output
make locks work even when curproc==0
(still crashes in clock interrupt)
  • Loading branch information
rtm committed Jul 12, 2006
1 parent 6643247 commit 8148b6e
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 32 deletions.
4 changes: 4 additions & 0 deletions Notes
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ in general, the table locks protect both free-ness and

why can't i get a lock in console code?
always triple fault
because release turns on interrupts!
a bad idea very early in main()
but mp_init() calls cprintf

lock code shouldn't call cprintf...
ide_init doesn't work now?
and IOAPIC: read from unsupported address
Expand Down
9 changes: 7 additions & 2 deletions console.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "spinlock.h"

struct spinlock console_lock;
int use_printf_lock = 0;

/*
* copy console output to parallel port, which you can tell
Expand All @@ -29,7 +30,8 @@ cons_putc(int c)
unsigned short *crt = (unsigned short *) 0xB8000; // base of CGA memory
int ind;

//acquire(&console_lock);
if(use_printf_lock)
acquire(&console_lock);

lpt_putc(c);

Expand Down Expand Up @@ -62,7 +64,8 @@ cons_putc(int c)
outb(crtport, 15);
outb(crtport + 1, ind);

//release(&console_lock);
if(use_printf_lock)
release(&console_lock);
}

void
Expand Down Expand Up @@ -127,6 +130,8 @@ cprintf(char *fmt, ...)
void
panic(char *s)
{
use_printf_lock = 0;
cprintf("panic: ");
cprintf(s, 0);
cprintf("\n", 0);
while(1)
Expand Down
3 changes: 3 additions & 0 deletions defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ void wakeup(void *);
void scheduler(void);
void proc_exit(void);
void yield(void);
void cli(void);
void sti(void);

// swtch.S
struct jmpbuf;
Expand Down Expand Up @@ -46,6 +48,7 @@ void pic_init(void);

// mp.c
void mp_init(void);
void mp_startthem(void);
int cpu(void);
int mp_isbcpu(void);
void lapic_init(int);
Expand Down
3 changes: 3 additions & 0 deletions kalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include "param.h"
#include "types.h"
#include "defs.h"
#include "param.h"
#include "mmu.h"
#include "proc.h"
#include "spinlock.h"

struct spinlock kalloc_lock;
Expand Down
13 changes: 12 additions & 1 deletion main.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,41 @@
#include "syscall.h"
#include "elf.h"
#include "param.h"
#include "spinlock.h"

extern char edata[], end[];
extern int acpu;
extern char _binary_user1_start[], _binary_user1_size[];
extern char _binary_usertests_start[], _binary_usertests_size[];
extern char _binary_userfs_start[], _binary_userfs_size[];

extern use_printf_lock;

int
main()
{
struct proc *p;

if (acpu) {
cpus[cpu()].clis = 1;
cprintf("an application processor\n");
idtinit(); // CPU's idt
lapic_init(cpu());
lapic_timerinit();
lapic_enableintr();
sti();
scheduler();
}
acpu = 1;

// clear BSS
memset(edata, 0, end - edata);

mp_init(); // just set up apic so cpu() works
use_printf_lock = 1;

cpus[cpu()].clis = 1; // cpu starts as if we had called cli()

cprintf("\nxV6\n\n");

pic_init(); // initialize PIC
Expand All @@ -56,7 +67,7 @@ main()
p->ppid = 0;
setupsegs(p);

mp_init(); // multiprocessor
mp_startthem();

// turn on timer and enable interrupts on the local APIC
lapic_timerinit();
Expand Down
11 changes: 8 additions & 3 deletions mp.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,6 @@ mp_init()
struct MPCTB *mpctb;
struct MPPE *proc;
struct MPBE *bus;
int c;
extern int main();
int i;

ncpu = 0;
Expand Down Expand Up @@ -386,13 +384,20 @@ mp_init()

lapic_init(bcpu-cpus);
cprintf("ncpu: %d boot %d\n", ncpu, bcpu-cpus);
}

void
mp_startthem()
{
extern uint8_t _binary_bootother_start[], _binary_bootother_size[];
extern int main();
int c;

memmove((void *) APBOOTCODE,_binary_bootother_start,
(uint32_t) _binary_bootother_size);

for(c = 0; c < ncpu; c++){
if (cpus+c == bcpu) continue;
if (c == cpu()) continue;
cprintf ("starting processor %d\n", c);
*(unsigned *)(APBOOTCODE-4) = (unsigned) (cpus[c].mpstack) + MPSTACK; // tell it what to use for %esp
*(unsigned *)(APBOOTCODE-8) = (unsigned)&main; // tell it where to jump to
Expand Down
25 changes: 23 additions & 2 deletions proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ scheduler(void)

if(i < NPROC){
np->state = RUNNING;
release(&proc_table_lock);
break;
}

Expand All @@ -157,8 +158,6 @@ scheduler(void)

cpus[cpu()].lastproc = np;
curproc[cpu()] = np;

release(&proc_table_lock);

// h/w sets busy bit in TSS descriptor sometimes, and faults
// if it's set in LTR. so clear tss descriptor busy bit.
Expand Down Expand Up @@ -252,3 +251,25 @@ proc_exit()
// switch into scheduler
swtch(ZOMBIE);
}

// disable interrupts
void
cli(void)
{
cpus[cpu()].clis += 1;
if(cpus[cpu()].clis == 1)
__asm __volatile("cli");
}

// enable interrupts
void
sti(void)
{
if(cpus[cpu()].clis < 1){
cprintf("cpu %d clis %d\n", cpu(), cpus[cpu()].clis);
panic("sti");
}
cpus[cpu()].clis -= 1;
if(cpus[cpu()].clis < 1)
__asm __volatile("sti");
}
1 change: 1 addition & 0 deletions proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ struct cpu {
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)
int clis; // cli() nesting depth
};

extern struct cpu cpus[NCPU];
Expand Down
32 changes: 23 additions & 9 deletions spinlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,58 @@

#define DEBUG 0

extern use_printf_lock;

int getcallerpc(void *v) {
return ((int*)v)[-1];
}

void
acquire(struct spinlock * lock)
{
struct proc * cp = curproc[cpu()];
unsigned who;

if(curproc[cpu()])
who = (unsigned) curproc[cpu()];
else
who = cpu() + 1;

// on a real machine there would be a memory barrier here
if(DEBUG) cprintf("cpu%d: acquiring at %x\n", cpu(), getcallerpc(&lock));
if (cp && lock->p == cp && lock->locked){

if (lock->who == who && lock->locked){
lock->count += 1;
} else {
cli();
while ( cmpxchg(0, 1, &lock->locked) != 1 ) { ; }
// if we get the lock, eax will be zero
// if we don't get the lock, eax will be one
while ( cmpxchg(0, 1, &lock->locked) == 1 ) { ; }
lock->locker_pc = getcallerpc(&lock);
lock->count = 1;
lock->p = cp;
lock->who = who;
}

if(DEBUG) cprintf("cpu%d: acquired at %x\n", cpu(), getcallerpc(&lock));
}

void
release(struct spinlock * lock)
{
struct proc * cp = curproc[cpu()];
unsigned who;

if(curproc[cpu()])
who = (unsigned) curproc[cpu()];
else
who = cpu() + 1;

if(DEBUG) cprintf ("cpu%d: releasing at %x\n", cpu(), getcallerpc(&lock));

if(lock->p != cp || lock->count < 1 || lock->locked != 1)
if(lock->who != who || lock->count < 1 || lock->locked != 1)
panic("release");

lock->count -= 1;
if(lock->count < 1){
lock->p = 0;
lock->who = 0;
cmpxchg(1, 0, &lock->locked);
sti();
// on a real machine there would be a memory barrier here
}
}
2 changes: 1 addition & 1 deletion spinlock.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
struct spinlock {
unsigned int locked;
struct proc *p;
unsigned who;
int count;
unsigned locker_pc;
};
14 changes: 0 additions & 14 deletions x86.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,20 +304,6 @@ read_tsc(void)
return tsc;
}

// disable interrupts
static __inline void
cli(void)
{
__asm __volatile("cli");
}

// enable interrupts
static __inline void
sti(void)
{
__asm __volatile("sti");
}

struct PushRegs {
/* registers as pushed by pusha */
uint32_t reg_edi;
Expand Down

0 comments on commit 8148b6e

Please sign in to comment.