Re: [PATCH] Futexes IV (Fast Lightweight Userspace Semaphores)

["Peter Wächtler" <pwaechtler@loewe-komp.de>]

Rusty Russell wrote:

> In message <3C9E1A10.F7AA6D6E@loewe-komp.de> you write:
> 
>>I can't see a reason why the ack-futex is needed. I think we can simply
>>delete it.
>>When deleted, the broadcast wouldn't block on ack (also preventing 
>>schedule ping-pong). With the cond->lock it's save to have several
>>broadcasters. That's fine.
>>
> 
> No, you might end up waking someone who did the pthread_cond_wait()
> after you did the pthread_cond_broadcast in place of one of the
> existing pthread_cond_wait() threads.
> 
> I don't believe this is allowed.  
> 


Indeed, I suspect that this isn't wanted.
With the cond->lock you almost prevent this: an ongoing broadcast
can't be intermixed with newly incoming waiters (they will block
on futex_down(&cond->lock))
But there is the window between a->b...


> 
>>But:
>>static int __pthread_cond_wait(pthread_cond_t *cond,
>>                               pthread_mutex_t *mutex,
>>                               const struct timespec *reltime)
>>{
>>        int ret;
>>
>>        /* Increment first so broadcaster knows we are waiting. */
>>	futex_down(&cond->lock);
>>        atomic_inc(cond->num_waiting);
>>(*)     futex_up(&mutex, 1);
>>a)	futex_up(&cond->lock, 1);  [move into syscall]
>>        do {
>>b)                ret = futex_down_time(&cond, ABSTIME); [cond_timed_wait]
>>        } while (ret < 0 && errno == EINTR);
>>	[futex_up(&cond->lock, 1); /* release condvar */]
>>
>>        futex_down(&mutex->futex);
>>        return ret;
>>}
>>
>>With the original code, we have a "signal/broadcast lost window (a->b)" 
>>that shouldn't be there:
>>
> 
> Where?  Having done the inc, the futex_up at (a) will fall through,
> giving the "thread behaves as if it [signal or broadcast] were issued
> after the about-to-block thread has blocked."
> 
Right after (a) another thread gets scheduled, issueing a signal/broadcast.

Ah, and then the futex_down_timed() wouldn't block, OK ;-)
But this way you have to use the ack->lock

 

I strongly believe, that the implementation of a condvar needs a lock
to prevent intermixed calls. You will see my comment on your implementation
with uwaitq. ;-)