futex: clean up fault logic
Impact: cleanup
Older versions of the futex code held the mmap_sem which had to
be dropped in order to call get_user(), so a two-pronged fault
handling mechanism was employed to handle faults of the atomic
operations. The mmap_sem is no longer held, so get_user()
should be adequate. This patch greatly simplifies the logic and
improves legibility.
Build and boot tested on a 4 way Intel x86_64 workstation.
Passes basic pthread_mutex and PI tests out of
ltp/testcases/realtime.
Signed-off-by: Darren Hart <dvhltc@us.ibm.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
LKML-Reference: <20090312075612.9856.48612.stgit@Aeon>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
diff --git a/kernel/futex.c b/kernel/futex.c
index c980a55..9c97f67 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -298,41 +298,6 @@
return ret ? -EFAULT : 0;
}
-/*
- * Fault handling.
- */
-static int futex_handle_fault(unsigned long address, int attempt)
-{
- struct vm_area_struct * vma;
- struct mm_struct *mm = current->mm;
- int ret = -EFAULT;
-
- if (attempt > 2)
- return ret;
-
- down_read(&mm->mmap_sem);
- vma = find_vma(mm, address);
- if (vma && address >= vma->vm_start &&
- (vma->vm_flags & VM_WRITE)) {
- int fault;
- fault = handle_mm_fault(mm, vma, address, 1);
- if (unlikely((fault & VM_FAULT_ERROR))) {
-#if 0
- /* XXX: let's do this when we verify it is OK */
- if (ret & VM_FAULT_OOM)
- ret = -ENOMEM;
-#endif
- } else {
- ret = 0;
- if (fault & VM_FAULT_MAJOR)
- current->maj_flt++;
- else
- current->min_flt++;
- }
- }
- up_read(&mm->mmap_sem);
- return ret;
-}
/*
* PI code:
@@ -760,9 +725,9 @@
struct futex_hash_bucket *hb1, *hb2;
struct plist_head *head;
struct futex_q *this, *next;
- int ret, op_ret, attempt = 0;
+ int ret, op_ret;
-retryfull:
+retry:
ret = get_futex_key(uaddr1, fshared, &key1);
if (unlikely(ret != 0))
goto out;
@@ -773,9 +738,8 @@
hb1 = hash_futex(&key1);
hb2 = hash_futex(&key2);
-retry:
double_lock_hb(hb1, hb2);
-
+retry_private:
op_ret = futex_atomic_op_inuser(op, uaddr2);
if (unlikely(op_ret < 0)) {
u32 dummy;
@@ -796,28 +760,16 @@
goto out_put_keys;
}
- /*
- * futex_atomic_op_inuser needs to both read and write
- * *(int __user *)uaddr2, but we can't modify it
- * non-atomically. Therefore, if get_user below is not
- * enough, we need to handle the fault ourselves, while
- * still holding the mmap_sem.
- */
- if (attempt++) {
- ret = futex_handle_fault((unsigned long)uaddr2,
- attempt);
- if (ret)
- goto out_put_keys;
- goto retry;
- }
-
ret = get_user(dummy, uaddr2);
if (ret)
goto out_put_keys;
+ if (!fshared)
+ goto retry_private;
+
put_futex_key(fshared, &key2);
put_futex_key(fshared, &key1);
- goto retryfull;
+ goto retry;
}
head = &hb1->chain;
@@ -877,6 +829,7 @@
hb1 = hash_futex(&key1);
hb2 = hash_futex(&key2);
+retry_private:
double_lock_hb(hb1, hb2);
if (likely(cmpval != NULL)) {
@@ -887,15 +840,16 @@
if (unlikely(ret)) {
double_unlock_hb(hb1, hb2);
+ ret = get_user(curval, uaddr1);
+ if (ret)
+ goto out_put_keys;
+
+ if (!fshared)
+ goto retry_private;
+
put_futex_key(fshared, &key2);
put_futex_key(fshared, &key1);
-
- ret = get_user(curval, uaddr1);
-
- if (!ret)
- goto retry;
-
- goto out_put_keys;
+ goto retry;
}
if (curval != *cmpval) {
ret = -EAGAIN;
@@ -1070,7 +1024,7 @@
struct futex_pi_state *pi_state = q->pi_state;
struct task_struct *oldowner = pi_state->owner;
u32 uval, curval, newval;
- int ret, attempt = 0;
+ int ret;
/* Owner died? */
if (!pi_state->owner)
@@ -1141,7 +1095,7 @@
handle_fault:
spin_unlock(q->lock_ptr);
- ret = futex_handle_fault((unsigned long)uaddr, attempt++);
+ ret = get_user(uval, uaddr);
spin_lock(q->lock_ptr);
@@ -1190,6 +1144,7 @@
if (unlikely(ret != 0))
goto out;
+retry_private:
hb = queue_lock(&q);
/*
@@ -1216,13 +1171,16 @@
if (unlikely(ret)) {
queue_unlock(&q, hb);
- put_futex_key(fshared, &q.key);
ret = get_user(uval, uaddr);
+ if (ret)
+ goto out_put_key;
- if (!ret)
- goto retry;
- goto out;
+ if (!fshared)
+ goto retry_private;
+
+ put_futex_key(fshared, &q.key);
+ goto retry;
}
ret = -EWOULDBLOCK;
if (unlikely(uval != val)) {
@@ -1356,7 +1314,7 @@
struct futex_hash_bucket *hb;
u32 uval, newval, curval;
struct futex_q q;
- int ret, lock_taken, ownerdied = 0, attempt = 0;
+ int ret, lock_taken, ownerdied = 0;
if (refill_pi_state_cache())
return -ENOMEM;
@@ -1376,7 +1334,7 @@
if (unlikely(ret != 0))
goto out;
-retry_unlocked:
+retry_private:
hb = queue_lock(&q);
retry_locked:
@@ -1601,18 +1559,15 @@
*/
queue_unlock(&q, hb);
- if (attempt++) {
- ret = futex_handle_fault((unsigned long)uaddr, attempt);
- if (ret)
- goto out_put_key;
- goto retry_unlocked;
- }
-
ret = get_user(uval, uaddr);
- if (!ret)
- goto retry_unlocked;
+ if (ret)
+ goto out_put_key;
- goto out_put_key;
+ if (!fshared)
+ goto retry_private;
+
+ put_futex_key(fshared, &q.key);
+ goto retry;
}
@@ -1628,7 +1583,7 @@
u32 uval;
struct plist_head *head;
union futex_key key = FUTEX_KEY_INIT;
- int ret, attempt = 0;
+ int ret;
retry:
if (get_user(uval, uaddr))
@@ -1644,7 +1599,6 @@
goto out;
hb = hash_futex(&key);
-retry_unlocked:
spin_lock(&hb->lock);
/*
@@ -1709,17 +1663,9 @@
* we have to drop the mmap_sem in order to call get_user().
*/
spin_unlock(&hb->lock);
-
- if (attempt++) {
- ret = futex_handle_fault((unsigned long)uaddr, attempt);
- if (ret)
- goto out;
- uval = 0;
- goto retry_unlocked;
- }
+ put_futex_key(fshared, &key);
ret = get_user(uval, uaddr);
- put_futex_key(fshared, &key);
if (!ret)
goto retry;
