Messages in this thread | | | Date | Fri, 27 Mar 2009 15:20:35 +0900 | Subject | Re: Question about PRIVATE_FUTEX | From | Minchan Kim <> |
| |
On Fri, Mar 27, 2009 at 2:50 PM, Eric Dumazet <dada1@cosmosbay.com> wrote: > Minchan Kim a écrit : >> Thanks for kind explanation. >> >> On Fri, Mar 27, 2009 at 1:56 PM, Eric Dumazet <dada1@cosmosbay.com> wrote: >>> Minchan Kim a écrit : >>>> Hi, Peter and Eric. >>>> >>>> I am not expert about futex. >>>> I am sorry if this is dumb question. >>>> >>>> If we use private futex, get_futex_key don't call get_user_pages_fast >>>> which pins page at page table. >>>> Then, get_futex_value_locked calls __cpy_from_user_inatomic with >>>> pagefault_disable. >>>> >>>> Who make sure the user page is mapped at app's page table ? >>>> >>> Nothing makes sure user page is mapped, as we dont have to (for private futexes >>> at least, since the 'key' is a combination of the futex virtual address (not >>> depending on the underlying physical page) and the task mm (sort of a static >>> offset per task) >>> If no page is mapped, a normal error should be returned to user, since >>> access to futex location will trigger a fault. >>> >> >> I mean as follows. >> It seems even shared futex case. >> >> After calling get_user_pages_fast, get_futex_key calls unlock_page and >> put_page, too. Then futex_wait calls get_futex_value_locked. >> >> Generally, current page->count is one and nolocked. >> I think kernel reclaimer can reclaim the page. >> >> Wouldn't kernel reclaim the page between get_fuex_key and >> get_futex_value_locked ? >> If kernel reclaimed the page, __copy_from_user_inatomic can happens >> page fault although pagefault_disable is on. >> >> How do we make sure this race condition ? >> Do I miss something ? >> > > Hmmm, so your question is not about PRIVATE futexes, but shared ones. > > I guess if page is no more present, its not a problem since > get_futex_value_locked() returns an error. We then take a slow > path, calling get_user() and retrying whole futex logic.
Indeed. I misunderstood about __copy_from_user_inatomic. It never sleep.
> However, comment at line 1213 is misleading I guess, since > we dont hold mmap semaphore anymore ? > > * for shared futexes, we hold the mmap semaphore, so the mapping > * cannot have changed since we looked it up in get_futex_key. > */ > ret = get_futex_value_locked(&uval, uaddr); > > So if page was un-mapped by another thread, and re-mapped to another physical > page, then this thread might sleep on 'kernel futex' not anymore reachable... > > User error, as it is not supposed to happen in a sane program, undefined > result...
Yes. How about removing confusing comments ?
Thanks for great explanation :)
-- Kinds regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |