On Sun, Sep 22, 2013 at 01:01:31AM +0200, Samuel Thibault wrote:
> Marin Ramesa, le Sat 21 Sep 2013 08:56:42 +0200, a écrit :
> > + case FUTEX_WAIT:
> > + return futex_wait(address);
> > + break;
>
>
> > + /* Save the value at the address. */
> > + int temp = *address;
> > +
> > + simple_lock(&futex_wait_lock);
> > +
> > + /* If the value after the lock is still the same. */
> > + if (temp == *address) {
>
> You have missed an important thing here: it's userland which provides
> the value we are supposed to check for. The idea is that there is a
> window between when userland sees the futex as seeming locked, and here.
> In the meanwhile some other thread might have unlocked the futex. And we
> don't want to hardcode in the kernel what a "locked futex" means. That's
> why it's userland which provides the value the kernel should check for.
>
> You can read "futexes are tricky" from Ulrich Drepper, for more details.
>
> > + /* Check if there is an existing futex at the address given. */
>
> Better use a hash table :)
Also, futex_wait_lock looks to be local to futex_wait...
--
Richard Braun
