Re: Poor condvar performance

Luke Elliott wrote:
> #define wake_up_all_sync(x) __wake_up_sync((x),TASK_UNINTERRUPTIBLE |
> TASK_INTERRUPTIBLE, 0)
> 
> and in kernel/sched.c:
> 
> void __wake_up_sync(wait_queue_head_t *q, unsigned int mode, int
> nr_exclusive)
> {
> 
> ...
> if (likely(nr_exclusive))
> 	__wake_up_common(q, mode, nr_exclusive, 1);
> else
> 	__wake_up_common(q, mode, nr_exclusive, 0);
> ...
> }
> 
> So it seems to me that wake_up_all_sync() is always calling
> __wake_up_common() with 0 for the sync flag (though I do not pretend to
> understand the behaviour of likely() / __builtin_expect so maybe that
> makes a difference).
> 
> If I change the __wake_up_sync() code to always pass 1 to
> __wake_up_common() (and keep the futex code calling wake_up_all_sync()
> ), I get far better (and repeatable) performance with context switches
> of the sensible order of 500 / second.

I think you have identified a bug in __wake_up_sync, because that
if(likely(nr_exclusive)) test doesn't correspond, as you noticed, with
the expectations of wake_up_all_sync().

Ingo, did you write that bit?  Do you know the reason for the if?

> Ulrich stated that "The best solution is to get requeue working
> (although two syscalls are made)." I would also appreciate some
> clarification of this - does it mean that the immediate scheduling of
> the woken thread would not happen, in a similar way to the hack to
> sched.c above? Because that seems to be a bad thing.

futex_requeue in the kernel should be working, and performing well,
since 2.6.0-test8.

Fixing the code to use futex_requeue should eliminate the useless
context switch pairs, shouldn't it?  Futex_requeue will simply move
the scheduled thread onto the queue of the mutex it would have blocked
on anyway, without waking that thread.

It won't defer single wakeups, though.  wake_up_all_sync() may still
be beneficial in some cases, though harmful in others.

(Perhaps FUTEX_WAKE_SYNC and FUTEX_REQUEUE_SYNC ops would be useful?)

-- Jamie