From ab08960f6402f5c7cbb7b6e81694a60b6abed4c8 Mon Sep 17 00:00:00 2001 From: rsc Date: Thu, 27 Sep 2007 21:25:37 +0000 Subject: Final word on the locking fiasco? Change pushcli / popcli so that they can never turn on interrupts unexpectedly. That is, if interrupts are on, then pushcli(); popcli(); turns them off and back on, but if they are off to begin with, then pushcli(); popcli(); is a no-op. I think our fundamental mistake was having a primitive (release and then popcli nee spllo) that could turn interrupts on at unexpected moments instead of being explicit about when we want to start allowing interrupts. With the new semantics, all the manual fiddling of ncli to force interrupts off in certain sections goes away. In return, we must explicitly mark the places where we want to enable interrupts unconditionally, by calling sti(). There is only one: inside the scheduler loop. --- spinlock.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'spinlock.c') diff --git a/spinlock.c b/spinlock.c index bf02292..a1aa37d 100644 --- a/spinlock.c +++ b/spinlock.c @@ -88,15 +88,19 @@ holding(struct spinlock *lock) } - -// XXX! -// Better names? Better functions? +// Pushcli/popcli are like cli/sti except that they are matched: +// it takes two popcli to undo two pushcli. Also, if interrupts +// are off, then pushcli, popcli leaves them off. void pushcli(void) { + int eflags; + + eflags = read_eflags(); cli(); - cpus[cpu()].ncli++; + if(cpus[cpu()].ncli++ == 0) + cpus[cpu()].intena = eflags & FL_IF; } void @@ -106,7 +110,7 @@ popcli(void) panic("popcli - interruptible"); if(--cpus[cpu()].ncli < 0) panic("popcli"); - if(cpus[cpu()].ncli == 0) + if(cpus[cpu()].ncli == 0 && cpus[cpu()].intena) sti(); } -- cgit v1.2.3