Re: [PATCH] Futex Generalization Patch

Rusty Russell (rusty@rustcorp.com.au)
Sat, 06 Apr 2002 19:48:11 +1000


In message <20020404162751.B0A253FE06@smtp.linux.ibm.com> you write:
> In futex_wait we have
> kmap(page)
> schedule_timeout()
> kunmap()

Oops! Good catch.,.. I've moved the kunmap to before the timeout...

> ---------------------
> A) in futex_down_timeout
> get ride of woken, don't see why you need that.
> optimize the while statement. Unless there are some hidden gcc issues.

We don't need to set to -1 if we never slept, that's why we have the
woken flag.

> static inline int futex_down_timeout(struct futex *futx, struct timespec *rel
)
> {
> int val, woken = 0;
>
> /* Returns new value */
> while ((val = __futex_down(&futx->count)) != 0) {
> switch (__futex_down_slow(futx, val, rel)) {
> case -1:
> return -1; /* error */
> case 0:
> futx->count = -1; /* slept */
> /* fall through */
> case 1:
> return 0; /* passed */
> }
> }
> }

case 0 does not return, it sleeps! This is wrong...

> Still missing something on the futex_trydown !!
>
> futex_trydown ::= futex_down == 1 ? 0 : -1
>
> So P1 holds the lock, P2 runs "while (1) { futex_trydown }" will decrement
> the counter yielding at some point "1" and thus granting the lock.
> At one GHz on 32 way system this only requires a lock hold time of a few
> seconds. Doesn't sound like a good idea.

Look closer at __futex_down: it doesn't decrement if futx->count < 0.

> This brings back the discussion on compare and swap. This would be trivial to
> do with compare and swap.

Yes, this is what the PPC code does.

Cheers,
Rusty.

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/