Commit aebeba09 authored by Jamie Lokier's avatar Jamie Lokier Committed by Linus Torvalds

[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: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 4846b054
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
* (C) Copyright 2003 Red Hat Inc, All Rights Reserved * (C) Copyright 2003 Red Hat Inc, All Rights Reserved
* *
* Removed page pinning, fix privately mapped COW pages and other cleanups * 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 * Thanks to Ben LaHaise for yelling "hashed waitqueues" loudly
* enough at me, Linus for the original (flawed) idea, Matthew * 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) ...@@ -486,22 +486,37 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time)
if (unlikely(ret != 0)) if (unlikely(ret != 0))
goto out_release_sem; 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 * 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) { if (get_user(curval, (int __user *)uaddr) != 0) {
ret = -EFAULT; ret = -EFAULT;
goto out_release_sem; goto out_unqueue;
} }
if (curval != val) { if (curval != val) {
ret = -EWOULDBLOCK; 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 * Now the futex is queued and we have checked the data, we
* don't want to hold mmap_sem while we sleep. * 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) ...@@ -542,10 +557,11 @@ static int futex_wait(unsigned long uaddr, int val, unsigned long time)
WARN_ON(!signal_pending(current)); WARN_ON(!signal_pending(current));
return -EINTR; return -EINTR;
out_unqueue:
/* If we were woken (and unqueued), we succeeded, whatever. */ /* If we were woken (and unqueued), we succeeded, whatever. */
if (!unqueue_me(&q)) if (!unqueue_me(&q))
ret = 0; ret = 0;
out_release_sem: out_release_sem:
up_read(&current->mm->mmap_sem); up_read(&current->mm->mmap_sem);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment