From: Rusty Russell Changes: (1) don't return 0 from futex_wait if we are somehow spuriously woken up, return -EINTR on any such case, (2) remove bogus comment about address no longer being in this address space: we hold the mm lock, and __pin_page succeeded, so it can't be true, (3) remove bogus comment about "get_user might fault and schedule", (4) remove list_empty check: we still hold the lock, so it can never happen. (5) Single error exit path, and move __queue_me to the end (order doesn't matter since we're inside the futex lock). kernel/futex.c | 53 ++++++++++++++++++++++++----------------------------- 1 files changed, 24 insertions(+), 29 deletions(-) diff -puN kernel/futex.c~futex-minor-fixes kernel/futex.c --- 25/kernel/futex.c~futex-minor-fixes 2003-08-25 20:33:00.000000000 -0700 +++ 25-akpm/kernel/futex.c 2003-08-25 20:33:00.000000000 -0700 @@ -312,7 +312,7 @@ static inline int futex_wait(unsigned lo unsigned long time) { DECLARE_WAITQUEUE(wait, current); - int ret = 0, curval; + int ret, curval; struct page *page; struct futex_q q; @@ -322,55 +322,50 @@ static inline int futex_wait(unsigned lo page = __pin_page(uaddr - offset); if (!page) { - unlock_futex_mm(); - return -EFAULT; + ret = -EFAULT; + goto unlock; } - __queue_me(&q, page, uaddr, offset, -1, NULL); - /* - * Page is pinned, but may no longer be in this address space. + * Page is pinned, but may be a kernel address. * It cannot schedule, so we access it with the spinlock held. */ if (get_user(curval, (int *)uaddr) != 0) { - unlock_futex_mm(); ret = -EFAULT; - goto out; + goto unlock; } + if (curval != val) { - unlock_futex_mm(); ret = -EWOULDBLOCK; - goto out; + goto unlock; } - /* - * The get_user() above might fault and schedule so we - * cannot just set TASK_INTERRUPTIBLE state when queueing - * ourselves into the futex hash. This code thus has to - * rely on the futex_wake() code doing a wakeup after removing - * the waiter from the list. - */ + + __queue_me(&q, page, uaddr, offset, -1, NULL); add_wait_queue(&q.waiters, &wait); set_current_state(TASK_INTERRUPTIBLE); - if (!list_empty(&q.list)) { - unlock_futex_mm(); - time = schedule_timeout(time); - } + unlock_futex_mm(); + + time = schedule_timeout(time); + set_current_state(TASK_RUNNING); /* * NOTE: we don't remove ourselves from the waitqueue because * we are the only user of it. */ - if (time == 0) { - ret = -ETIMEDOUT; - goto out; - } - if (signal_pending(current)) - ret = -EINTR; -out: - /* Were we woken up anyway? */ + + /* Were we woken up (and removed from queue)? Always return + * success when this happens. */ if (!unqueue_me(&q)) ret = 0; + else if (time == 0) + ret = -ETIMEDOUT; + else + ret = -EINTR; + put_page(q.page); + return ret; +unlock: + unlock_futex_mm(); return ret; } _