diff options
author | rsc <rsc> | 2007-09-30 14:30:04 +0000 |
---|---|---|
committer | rsc <rsc> | 2007-09-30 14:30:04 +0000 |
commit | 9fd9f80431ad85552c0969831a3ccc3e800ac464 (patch) | |
tree | e9190c3029d9b3eda767ae9d2cb93e0e54344e7b | |
parent | c840f3ecdc718c4a6eb6fbd9e0cbb0a012c3adf4 (diff) | |
download | xv6-labs-9fd9f80431ad85552c0969831a3ccc3e800ac464.tar.gz xv6-labs-9fd9f80431ad85552c0969831a3ccc3e800ac464.tar.bz2 xv6-labs-9fd9f80431ad85552c0969831a3ccc3e800ac464.zip |
Re: why cpuid() in locking code?
rtm wrote:
> Why does acquire() call cpuid()? Why does release() call cpuid()?
The cpuid in acquire is redundant with the cmpxchg, as you said.
I have removed the cpuid from acquire.
The cpuid in release is actually doing something important,
but not on the hardware. It keeps gcc from reordering the
lock->locked assignment above the other two during optimization.
(Not that current gcc -O2 would choose to do that, but it is allowed to.)
I have replaced the cpuid in release with a "gcc barrier" that
keeps gcc from moving things around but has no hardware effect.
On a related note, I don't think the cpuid in mpmain is necessary,
for the same reason that the cpuid wasn't needed in release.
As to the question of whether
acquire();
x = protected;
release();
might read protected after release(), I still haven't convinced
myself whether it can. I'll put the cpuid back into release if
we determine that it can.
Russ
-rw-r--r-- | main.c | 1 | ||||
-rw-r--r-- | spinlock.c | 16 | ||||
-rw-r--r-- | x86.h | 1 |
3 files changed, 9 insertions, 9 deletions
@@ -50,7 +50,6 @@ mpmain(void) if(cpu() != mp_bcpu()) lapic_init(cpu()); setupsegs(0); - cpuid(0, 0, 0, 0, 0); // memory barrier cpus[cpu()].booted = 1; scheduler(); @@ -10,6 +10,12 @@ extern int use_console_lock; +// Barrier to gcc's instruction reordering. +static void inline gccbarrier(void) +{ + asm volatile("" : : : "memory"); +} + void initlock(struct spinlock *lock, char *name) { @@ -32,10 +38,6 @@ acquire(struct spinlock *lock) while(cmpxchg(0, 1, &lock->locked) == 1) ; - // Serialize instructions: now that lock is acquired, make sure - // we wait for all pending writes from other processors. - cpuid(0, 0, 0, 0, 0); // memory barrier (see Ch 7, IA-32 manual vol 3) - // Record info about lock acquisition for debugging. // The +10 is only so that we can tell the difference // between forgetting to initialize lock->cpu @@ -53,12 +55,10 @@ release(struct spinlock *lock) lock->pcs[0] = 0; lock->cpu = 0xffffffff; - - // Serialize instructions: before unlocking the lock, make sure - // to flush any pending memory writes from this processor. - cpuid(0, 0, 0, 0, 0); // memory barrier (see Ch 7, IA-32 manual vol 3) + gccbarrier(); // Keep gcc from moving lock->locked = 0 earlier. lock->locked = 0; + popcli(); } @@ -96,6 +96,7 @@ write_eflags(uint eflags) asm volatile("pushl %0; popfl" : : "r" (eflags)); } +// XXX: Kill this if not used. static inline void cpuid(uint info, uint *eaxp, uint *ebxp, uint *ecxp, uint *edxp) { |