Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH] Futex Generalization Patch | Date | 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/
| |