Re: [PATCH v3 4/4] futex: Avoid taking hb lock if nothing to wakeup

[Linus Torvalds]

On Thu, Dec 19, 2013 at 3:42 PM, Davidlohr Bueso <davidlohr@xxxxxx> wrote:
>
>> And it can look at
>> the wait-list for that - but you want to close the race between the
>> entry actually getting added to the list using this counter. But the
>> place you increment the new counter is the same place as you take the
>> spinlock, which does that ticket increment. No?
>
> I don't think so. If we rely on this, then we could end up missing
> to-be-queued tasks that are in the process of acquiring the lock, so
> waiters could sleep forever. So we need a way of acknowledging that a
> task is in the process of waiting when concurrently doing wakeups.

I don't understand. Why is that any better with the separate atomic count?

The only place you increment the count - hb_waiters_inc() - is called
in exactly two places:

 - in requeue_futex(), which already holds the spinlock on both queues

 - in queue_lock(), immediately before getting the spinlock (which
will do the SAME ATOMIC INCREMENT, except it's just doing it on a
different member of the structure, namely the spinlock head)

so if you drop the waiters increment, and instead use the spinlock
head increment as the "waiters", you get exactly the same semantics.
If anything, for the requeue_futex() case, the spinlock increment was
done *earlier*, but that's really not relevant.

So how could we miss this? Explain to me what the separate counter
does that isn't done by the spinlock head counter.

              Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/