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

[Linus Torvalds]

On Thu, Dec 19, 2013 at 10:45 AM, Davidlohr Bueso <davidlohr@xxxxxx> wrote:
>
> - increment the counter at queue_lock() as we always end up calling
>   queue_me() which adds the element to the list. Upon any error,
>   queue_unlock() is called for housekeeping, for which we decrement
>   to mach the increment done in queue_lock().
>
> - decrement the counter at __unqueue_me() to reflect when an element is
>   removed from the queue for wakeup related purposes.

I still hate this whole separate counter thing. It seems really annoying.

If re-ordering things didn't work out, then why can't just the counter
we *already* have in the spinlock itself work as the counter? Your
counter update logic seems to basically match when you take the
spinlock anyway.

The *testing* side doesn't actually care about how many waiters there
are, it only cares about whether there are waiters. 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?

So I still think this can be done without that new counter field, or
any new atomics.

hb_waiters_pending() could be something like

    spin_contended(hb->lock) || !list_empty(&hb->chain)

which considering where you increment the new count should be
equivalent to your "!!hb->waiters". The smp_mb_after_atomic_inc()" on
the spinlock side would become smp_mb_after_spin_lock() instead.

Yes? No? Why?

            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/