Messages in this thread | | | Date | Fri, 19 Dec 2008 23:37:20 +0100 | From | Ingo Molnar <> | Subject | Re: futex.c and fault handling |
| |
(extended the Cc: list with MM experts.)
* Darren Hart <dvhltc@us.ibm.com> wrote:
> I've been working in linux-tip core/futexes lately and have a need to be > able to properly handle faults for r/w access to a uaddr. I was > planning on modeling this on the fault handling in futex_lock_pi which > used both get_user() and futex_handle_fault() to get the pages. > However, that used to be based on whether or not we held the mmap_sem. > Now that we're using fast_gup throughout futex.c, and the mmap_sem > locking has been pushed in tighter in get_futex_key(), I'm not sure if > the fault handling is still correct - the comments are certainly > incorrect since we no longer hold the mmap_sem when we hit > uaddr_faulted: inside futex_lock_pi (and a few other places have similar > comment vs. code dicrepancies): > > uaddr_faulted: > /* > * We have to r/w *(int __user *)uaddr, and we have to modify it > * atomically. Therefore, if we continue to fault after get_user() > * below, we need to handle the fault ourselves, while still holding > * the mmap_sem. This can occur if the uaddr is under contention as > * we have to drop the mmap_sem in order to call get_user(). > */ > queue_unlock(&q, hb); > > if (attempt++) { > ret = futex_handle_fault((unsigned long)uaddr, attempt); > if (ret) > goto out_put_key; > goto retry_unlocked; > } > > ---> previous versions dropped the mmap_sem here in preparation for get_user() > > ret = get_user(uval, uaddr); > if (!ret) > goto retry; > > > So is the code still correct without the holding of mmap_sem? I suppose > get_user() is still the more efficient path, and perhaps even more so > now that we don't have to release mmap_sem and reacquire it later in > order to call it. If so, then I guess all that is needed is a comments > patch, which I'd be happy to write up. > > Thanks, > > -- > Darren Hart > IBM Linux Technology Center > Real-Time Linux Team
| |