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

[Martin Wirth]

Rusty Russell wrote:

> 1) Where this is flawed,

I. There is a race in __pthread_cond_wait between timeout and a 
cond_signal or broadcast. If the signal comes in

} while (ret < 0 && errno == EINTR);
 >>>>> we leave with errno==ETIMEDOUT and get signal or broadcast called 
here
	if (atomic_dec_and_test(cond->num_waiting))

then you up cond->wait one time to often, leaving it in an invalid state.

II. Your implementation relies on the fact that the signal or broadcast
caller owns the mutex used in cond_wait. According to the POSIX spec 
this need not be the case. The only thing that may happen is that you
miss a wakeup. But it is not allowed to screw up the internal state of
of the condition variable, which might well happen in your 
implementation. (Note: Calling cond_signal without holding the mutex is 
not necessarily flawed software. Think of a periodically occurring 
new_data or data_changed flag where it is not really important to sleep 
race free)

III. Minor nit: You should also clear cond->ack.count
in cond_signal otherwise it may wrap around soon (at least for a
24-bit atomic variable) if you mostly use cond_signal.

> 2) Where this is suboptimal,

As said in a previous e-mail, you need an futex_up(..,n) that
really wakes_up n thread at once.

> 3) What kernel primitive would help to resolve these?

Your exported waitqueues or my suggestion for a second waitqueue 
associated with a futex.

Martin