Messages in this thread | | | Date | Thu, 23 Jun 2011 08:11:01 -0700 | From | Darren Hart <> | Subject | Re: [RFC PATCH] futex: replace get_user_pages() with get_user_pages_fast() |
| |
On 06/22/2011 11:21 PM, KOSAKI Motohiro wrote: > Hello, > > Is there any reason to take mmap_sem explicitly here? > Should we change gup_fast() to allow NULL argument (ie for avoid get_page)? > > ============================ > down_read(&mm->mmap_sem); > get_user_pages(current, current->mm) > up_read(&mm->mmap_sem); > > and > > get_user_pages_fast() > > make an equivalent result, And latter would be better when mamp_sem > highly contended case, because it can avoid to take mmap_sem if > the target page doesn't need a page fault.
I can't speak authoritatively here, but it seems to me that get_user_pages_fast falls back to get_user_pages with mmap_sem anyway, so this seems like a reasonable optimization for the best case, with a minor overhead for the slow case.
Peter, am I missing something? Is there a reason you left this as is during your fast gup futex update?
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > --- > kernel/futex.c | 8 +++----- > 1 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/kernel/futex.c b/kernel/futex.c > index fe28dc2..9f7b1ae 100644 > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -351,13 +351,11 @@ static inline void put_futex_key(union futex_key *key) > */ > static int fault_in_user_writeable(u32 __user *uaddr) > { > - struct mm_struct *mm = current->mm; > int ret; > + struct page *page; > > - down_read(&mm->mmap_sem); > - ret = get_user_pages(current, mm, (unsigned long)uaddr, > - 1, 1, 0, NULL, NULL); > - up_read(&mm->mmap_sem); > + ret = get_user_pages_fast((unsigned long)uaddr, 1, 1, &page); > + put_page(page); > > return ret < 0 ? ret : 0; > }
-- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
| |