Messages in this thread Patch in this message |  | | Date | Sat, 1 Oct 2016 21:12:09 -0400 | From | Waiman Long <> | Subject | Re: [PATCH v3 07/13] futex: Throughput-optimized (TP) futexes |
| |
On 10/01/2016 02:47 AM, Thomas Gleixner wrote: > On Fri, 30 Sep 2016, Waiman Long wrote: >> + WRITE_ONCE(state->owner, current); >> + preempt_disable(); >> + for (;;) { >> + ret = futex_trylock(uaddr, vpid,&uval, true); > Did you actually read what I said? You CANNOT access userspace in a preempt > disabled region without disabling pagefaults and handle the resulting > wreckage yourself.
I think I had missed that comment. My bad:-(
I will fix that with the code changes below. I will also double-check your comments again to see if I miss some others.
Cheers, Longman
---------------------------[ cut here ]-----------------------------------------
diff --git a/kernel/futex.c b/kernel/futex.c index bc16eca..132a36d 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -3520,6 +3520,13 @@ static int futex_spin_on_owner(u32 __user *uaddr, u32 vpid, bool on_owner_pi_list = false;
WRITE_ONCE(state->owner, current); +retry: + /* + * The preempt_disable() has similar effect as pagefault_disable(). + * As a result, we will have to disable page fault as well and handle + * the case of faulting in the futex word. + */ + pagefault_disable(); preempt_disable(); for (;; loop--) { ret = futex_trylock(uaddr, vpid, &uval, true); @@ -3648,6 +3655,14 @@ static int futex_spin_on_owner(u32 __user *uaddr, u32 vpid, } out: preempt_enable(); + pagefault_enable(); + + if (ret == -EFAULT) { + ret = fault_in_user_writeable(uaddr); + if (!ret) + goto retry; + } + if (owner_task) { if (on_owner_pi_list) task_pi_list_del(owner_task, state, false);
|  |