diff options
author | Jamie Lokier <jamie@shareable.org> | 2004-11-14 02:57:08 -0800 |
---|---|---|
committer | Linus Torvalds <torvalds@ppc970.osdl.org> | 2004-11-14 02:57:08 -0800 |
commit | aebeba096859f0e6c9b35e04a2b4b9e9a56736b2 (patch) | |
tree | e5344904f559bd44d8dc26fa6ade26008736f4f4 /kernel | |
parent | 4846b054fa309bfaeb241b829c26e3b13b7d06ba (diff) | |
download | history-aebeba096859f0e6c9b35e04a2b4b9e9a56736b2.tar.gz |
[PATCH] revert recent futex_wait fix
The patch was wrong. Back it out, and add some commentary explaining why we
need to run queue_me() prior to the get_user().
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'kernel')
-rw-r--r-- | kernel/futex.c | 32 |
1 files changed, 24 insertions, 8 deletions
diff --git a/kernel/futex.c b/kernel/futex.c index efaa4bcc1a383b..fc9b739e675cd6 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -6,7 +6,7 @@ * (C) Copyright 2003 Red Hat Inc, All Rights Reserved * * Removed page pinning, fix privately mapped COW pages and other cleanups - * (C) Copyright 2003 Jamie Lokier + * (C) Copyright 2003, 2004 Jamie Lokier * * Thanks to Ben LaHaise for yelling "hashed waitqueues" loudly * enough at me, Linus for the original (flawed) idea, Matthew @@ -486,22 +486,37 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time) if (unlikely(ret != 0)) goto out_release_sem; + queue_me(&q, -1, NULL); + /* - * Access the page after the futex is queued. + * Access the page AFTER the futex is queued. + * Order is important: + * + * Userspace waiter: val = var; if (cond(val)) futex_wait(&var, val); + * Userspace waker: if (cond(var)) { var = new; futex_wake(&var); } + * + * The basic logical guarantee of a futex is that it blocks ONLY + * if cond(var) is known to be true at the time of blocking, for + * any cond. If we queued after testing *uaddr, that would open + * a race condition where we could block indefinitely with + * cond(var) false, which would violate the guarantee. + * + * A consequence is that futex_wait() can return zero and absorb + * a wakeup when *uaddr != val on entry to the syscall. This is + * rare, but normal. + * * We hold the mmap semaphore, so the mapping cannot have changed - * since we looked it up. + * since we looked it up in get_futex_key. */ if (get_user(curval, (int __user *)uaddr) != 0) { ret = -EFAULT; - goto out_release_sem; + goto out_unqueue; } if (curval != val) { ret = -EWOULDBLOCK; - goto out_release_sem; + goto out_unqueue; } - queue_me(&q, -1, NULL); - /* * Now the futex is queued and we have checked the data, we * don't want to hold mmap_sem while we sleep. @@ -542,10 +557,11 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time) WARN_ON(!signal_pending(current)); return -EINTR; + out_unqueue: /* If we were woken (and unqueued), we succeeded, whatever. */ if (!unqueue_me(&q)) ret = 0; -out_release_sem: + out_release_sem: up_read(¤t->mm->mmap_sem); return ret; } |