Re: [PATCH] bug in futex unqueue_me

From: Christian Borntraeger
Date: Mon Jul 31 2006 - 04:02:29 EST

On Sunday 30 July 2006 08:38, Ingo Molnar wrote:
> interesting, how is this possible? We do a spin_lock(lock_ptr), and
> taking a spinlock is an implicit barrier(). So gcc must not delay
> evaluating lock_ptr to inside the critical section. And as far as i can
> see the s390 spinlock implementation goes through an 'asm volatile'
> piece of code, which is a barrier already. So how could this have
> happened?

spin_lock is a barrier, but isnt the barrierness too late here? The compiler
reloads the value of lock_ptr after the "if(lock_ptr)" and *before* calling
3ee: e3 c0 b0 28 00 04 lg %r12,40(%r11)
q->lockptr in r12
3f4: b9 02 00 cc ltgr %r12,%r12
load and test r12
3f8: a7 84 00 4b je 48e <unqueue_me+0xc6>
if r12 == 0 jump away
3fc: e3 20 b0 28 00 04 lg %r2,40(%r11)
q->lockptr in r2
402: c0 e5 00 00 00 00 brasl %r14,402 <unqueue_me+0x3a>
404: R_390_PC32DBL _spin_lock+0x2
call spinlock (r2 is first parameter)

I really dont know why the compiler reloads lock_ptr from memory at all, but I
will talk to our compiler guys to find out.

> I have nothing against adding a barrier(), but we should first
> investigate why the spin_lock() didnt act as a barrier - there might be
> other, similar bugs hiding. (we rely on spin_lock()s barrier-ness in a
> fair number of places)
See above. I think the barrier must be before "if(lock_ptr)" and not

> yes, it is always a pointer to a valid spinlock, or NULL.
> futex_requeue() can change the spinlock from one to another, and
> wake_futex() can change it to NULL. The futex unqueue_me() fastpath is
> when a futex waiter was woken - in which case it's NULL. But it can
> still be non-NULL if we timed out or a signal happened, in which case we
> may race with a wakeup or a requeue. futex_requeue() changes the
> spinlock pointer if it holds both the old and the new spinlock. So it's
> race-free as far as i can see.
Ok, looks fine then.

Mit freundlichen Grüßen / Best Regards

Christian Borntraeger
Linux Software Engineer zSeries Linux & Virtualization

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at