i think my cmpxchg use was wrong in acquire

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)
This commit is contained in:
rtm 2006-07-12 11:15:38 +00:00
parent 664324745e
commit 8148b6ee53
11 changed files with 85 additions and 32 deletions

4
Notes
View file

@ -125,6 +125,10 @@ in general, the table locks protect both free-ness and
why can't i get a lock in console code? why can't i get a lock in console code?
always triple fault 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... lock code shouldn't call cprintf...
ide_init doesn't work now? ide_init doesn't work now?
and IOAPIC: read from unsupported address and IOAPIC: read from unsupported address

View file

@ -4,6 +4,7 @@
#include "spinlock.h" #include "spinlock.h"
struct spinlock console_lock; struct spinlock console_lock;
int use_printf_lock = 0;
/* /*
* copy console output to parallel port, which you can tell * copy console output to parallel port, which you can tell
@ -29,7 +30,8 @@ cons_putc(int c)
unsigned short *crt = (unsigned short *) 0xB8000; // base of CGA memory unsigned short *crt = (unsigned short *) 0xB8000; // base of CGA memory
int ind; int ind;
//acquire(&console_lock); if(use_printf_lock)
acquire(&console_lock);
lpt_putc(c); lpt_putc(c);
@ -62,7 +64,8 @@ cons_putc(int c)
outb(crtport, 15); outb(crtport, 15);
outb(crtport + 1, ind); outb(crtport + 1, ind);
//release(&console_lock); if(use_printf_lock)
release(&console_lock);
} }
void void
@ -127,6 +130,8 @@ cprintf(char *fmt, ...)
void void
panic(char *s) panic(char *s)
{ {
use_printf_lock = 0;
cprintf("panic: ");
cprintf(s, 0); cprintf(s, 0);
cprintf("\n", 0); cprintf("\n", 0);
while(1) while(1)

3
defs.h
View file

@ -19,6 +19,8 @@ void wakeup(void *);
void scheduler(void); void scheduler(void);
void proc_exit(void); void proc_exit(void);
void yield(void); void yield(void);
void cli(void);
void sti(void);
// swtch.S // swtch.S
struct jmpbuf; struct jmpbuf;
@ -46,6 +48,7 @@ void pic_init(void);
// mp.c // mp.c
void mp_init(void); void mp_init(void);
void mp_startthem(void);
int cpu(void); int cpu(void);
int mp_isbcpu(void); int mp_isbcpu(void);
void lapic_init(int); void lapic_init(int);

View file

@ -10,6 +10,9 @@
#include "param.h" #include "param.h"
#include "types.h" #include "types.h"
#include "defs.h" #include "defs.h"
#include "param.h"
#include "mmu.h"
#include "proc.h"
#include "spinlock.h" #include "spinlock.h"
struct spinlock kalloc_lock; struct spinlock kalloc_lock;

13
main.c
View file

@ -8,6 +8,7 @@
#include "syscall.h" #include "syscall.h"
#include "elf.h" #include "elf.h"
#include "param.h" #include "param.h"
#include "spinlock.h"
extern char edata[], end[]; extern char edata[], end[];
extern int acpu; extern int acpu;
@ -15,23 +16,33 @@ extern char _binary_user1_start[], _binary_user1_size[];
extern char _binary_usertests_start[], _binary_usertests_size[]; extern char _binary_usertests_start[], _binary_usertests_size[];
extern char _binary_userfs_start[], _binary_userfs_size[]; extern char _binary_userfs_start[], _binary_userfs_size[];
extern use_printf_lock;
int int
main() main()
{ {
struct proc *p; struct proc *p;
if (acpu) { if (acpu) {
cpus[cpu()].clis = 1;
cprintf("an application processor\n"); cprintf("an application processor\n");
idtinit(); // CPU's idt idtinit(); // CPU's idt
lapic_init(cpu()); lapic_init(cpu());
lapic_timerinit(); lapic_timerinit();
lapic_enableintr(); lapic_enableintr();
sti();
scheduler(); scheduler();
} }
acpu = 1; acpu = 1;
// clear BSS // clear BSS
memset(edata, 0, end - edata); 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"); cprintf("\nxV6\n\n");
pic_init(); // initialize PIC pic_init(); // initialize PIC
@ -56,7 +67,7 @@ main()
p->ppid = 0; p->ppid = 0;
setupsegs(p); setupsegs(p);
mp_init(); // multiprocessor mp_startthem();
// turn on timer and enable interrupts on the local APIC // turn on timer and enable interrupts on the local APIC
lapic_timerinit(); lapic_timerinit();

11
mp.c
View file

@ -325,8 +325,6 @@ mp_init()
struct MPCTB *mpctb; struct MPCTB *mpctb;
struct MPPE *proc; struct MPPE *proc;
struct MPBE *bus; struct MPBE *bus;
int c;
extern int main();
int i; int i;
ncpu = 0; ncpu = 0;
@ -386,13 +384,20 @@ mp_init()
lapic_init(bcpu-cpus); lapic_init(bcpu-cpus);
cprintf("ncpu: %d boot %d\n", ncpu, bcpu-cpus); cprintf("ncpu: %d boot %d\n", ncpu, bcpu-cpus);
}
void
mp_startthem()
{
extern uint8_t _binary_bootother_start[], _binary_bootother_size[]; extern uint8_t _binary_bootother_start[], _binary_bootother_size[];
extern int main();
int c;
memmove((void *) APBOOTCODE,_binary_bootother_start, memmove((void *) APBOOTCODE,_binary_bootother_start,
(uint32_t) _binary_bootother_size); (uint32_t) _binary_bootother_size);
for(c = 0; c < ncpu; c++){ for(c = 0; c < ncpu; c++){
if (cpus+c == bcpu) continue; if (c == cpu()) continue;
cprintf ("starting processor %d\n", c); cprintf ("starting processor %d\n", c);
*(unsigned *)(APBOOTCODE-4) = (unsigned) (cpus[c].mpstack) + MPSTACK; // tell it what to use for %esp *(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 *(unsigned *)(APBOOTCODE-8) = (unsigned)&main; // tell it where to jump to

25
proc.c
View file

@ -148,6 +148,7 @@ scheduler(void)
if(i < NPROC){ if(i < NPROC){
np->state = RUNNING; np->state = RUNNING;
release(&proc_table_lock);
break; break;
} }
@ -157,8 +158,6 @@ scheduler(void)
cpus[cpu()].lastproc = np; cpus[cpu()].lastproc = np;
curproc[cpu()] = np; curproc[cpu()] = np;
release(&proc_table_lock);
// h/w sets busy bit in TSS descriptor sometimes, and faults // h/w sets busy bit in TSS descriptor sometimes, and faults
// if it's set in LTR. so clear tss descriptor busy bit. // if it's set in LTR. so clear tss descriptor busy bit.
@ -252,3 +251,25 @@ proc_exit()
// switch into scheduler // switch into scheduler
swtch(ZOMBIE); 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
proc.h
View file

@ -69,6 +69,7 @@ struct cpu {
struct jmpbuf jmpbuf; struct jmpbuf jmpbuf;
char mpstack[MPSTACK]; // per-cpu start-up stack, only used to get into main() 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) struct proc *lastproc; // last proc scheduled on this cpu (never NULL)
int clis; // cli() nesting depth
}; };
extern struct cpu cpus[NCPU]; extern struct cpu cpus[NCPU];

View file

@ -8,6 +8,8 @@
#define DEBUG 0 #define DEBUG 0
extern use_printf_lock;
int getcallerpc(void *v) { int getcallerpc(void *v) {
return ((int*)v)[-1]; return ((int*)v)[-1];
} }
@ -15,37 +17,49 @@ int getcallerpc(void *v) {
void void
acquire(struct spinlock * lock) 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(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; lock->count += 1;
} else { } else {
cli(); 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->locker_pc = getcallerpc(&lock);
lock->count = 1; lock->count = 1;
lock->p = cp; lock->who = who;
} }
if(DEBUG) cprintf("cpu%d: acquired at %x\n", cpu(), getcallerpc(&lock)); if(DEBUG) cprintf("cpu%d: acquired at %x\n", cpu(), getcallerpc(&lock));
} }
void void
release(struct spinlock * lock) 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(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"); panic("release");
lock->count -= 1; lock->count -= 1;
if(lock->count < 1){ if(lock->count < 1){
lock->p = 0; lock->who = 0;
cmpxchg(1, 0, &lock->locked); cmpxchg(1, 0, &lock->locked);
sti(); sti();
// on a real machine there would be a memory barrier here
} }
} }

View file

@ -1,6 +1,6 @@
struct spinlock { struct spinlock {
unsigned int locked; unsigned int locked;
struct proc *p; unsigned who;
int count; int count;
unsigned locker_pc; unsigned locker_pc;
}; };

14
x86.h
View file

@ -304,20 +304,6 @@ read_tsc(void)
return tsc; return tsc;
} }
// disable interrupts
static __inline void
cli(void)
{
__asm __volatile("cli");
}
// enable interrupts
static __inline void
sti(void)
{
__asm __volatile("sti");
}
struct PushRegs { struct PushRegs {
/* registers as pushed by pusha */ /* registers as pushed by pusha */
uint32_t reg_edi; uint32_t reg_edi;