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
This commit is contained in:
rsc 2007-09-30 14:30:04 +00:00
parent c840f3ecdc
commit 9fd9f80431
3 changed files with 9 additions and 9 deletions

1
main.c
View file

@ -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();

View file

@ -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
@ -54,11 +56,9 @@ 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();
}

1
x86.h
View file

@ -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)
{