fast_gup for shared futexes

On the way of getting rid of the mmap_sem requirement for shared futexes, start by relying on get_user_pages().

This requires we get the page associated with the key, and put the page when we're done with it.

Quoted 5 lines · Peter Zijlstra

On the way of getting rid of the mmap_sem requirement for shared futexes, start by relying on get_user_pages().

This requires we get the page associated with the key, and put the page when we're done with it.

Hi Peter,

Cool.

I'm all for removing mmap_sem requirement from shared futexes... Are there many apps which make a non-trivial use of them I wonder?

I guess it will help legacy (pre-FUTEX_PRIVATE) usespaces in performance too, though.

What I'm worried about with this is invalidate or truncate races. With direct IO, it obviously doesn't matter because the only requirement is that the page existed at the address at some point during the syscall...

So I'd really like you to not carry the page around in the key, but just continue using the same key we have now. Also, lock the page and ensure it hasn't been truncated before taking the inode from the key and incrementing its count (page lock's extra atomics should be more or less cancelled out by fewer mmap_sem atomic ops).

get_futex_key should look something like this I would have thought:?

BTW. I like that it removes a lot of fshared crap from around the place. And also this is a really good user of fast_gup because I guess it should usually be faulted in. The problem is that this could be a little more expensive for architectures that don't implement fast_gup. Though most should be able to.

kernel/futex.c +13 -17

@@ -191,7 +191,6 @@ static int get_futex_key(u32 __user *uad
 {
        unsigned long address = (unsigned long)uaddr;
        struct mm_struct *mm = current->mm;
-       struct vm_area_struct *vma;
        struct page *page;
        int err;
 

@@ -210,27 +209,26 @@ static int get_futex_key(u32 __user *uad
         * Note : We do have to check 'uaddr' is a valid user address,
         *        but access_ok() should be faster than find_vma()
         */
-       if (!fshared) {
+       if (likely(!fshared)) {
                if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
                        return -EFAULT;
                key->private.mm = mm;
                key->private.address = address;
                return 0;
        }
-       /*
-        * The futex is hashed differently depending on whether
-        * it's in a shared or private mapping.  So check vma first.
-        */
-       vma = find_extend_vma(mm, address);
-       if (unlikely(!vma))
-               return -EFAULT;
-
-       /*
-        * Permissions.
-        */
-       if (unlikely((vma->vm_flags & (VM_IO|VM_READ)) != VM_READ))
-               return (vma->vm_flags & VM_IO) ? -EPERM : -EACCES;
 
+again:
+       err = fast_gup(address, 1, 0, &page);
+       if (err < 0)
+               return err;
+
+       lock_page(page);
+       if (!page->mapping) { /* PageAnon pages shouldn't get caught here */
+               unlock_page(page);
+               put_page(page);
+               goto again;
+       }
+
        /*
         * Private mappings are handled in a simple way.
         *

@@ -240,38 +238,19 @@ static int get_futex_key(u32 __user *uad
         * VM_MAYSHARE here, not VM_SHARED which is restricted to shared
         * mappings of _writable_ handles.
         */
-       if (likely(!(vma->vm_flags & VM_MAYSHARE))) {
-               key->both.offset |= FUT_OFF_MMSHARED; /* reference taken on mm 
 *

/

kernel/futex.c +5 -26

+       if (PageAnon(page)) {
+               key->both.offset |= FUT_OFF_MMSHARED; /* ref taken on mm */
                key->private.mm = mm;
                key->private.address = address;
-               return 0;
-       }
-
-       /*
-        * Linear file mappings are also simple.
-        */
-       key->shared.inode = vma->vm_file->f_path.dentry->d_inode;
-       key->both.offset |= FUT_OFF_INODE; /* inode-based key. */
-       if (likely(!(vma->vm_flags & VM_NONLINEAR))) {
-               key->shared.pgoff = (((address - vma->vm_start) >> PAGE_SHIFT)
-                                    + vma->vm_pgoff);
-               return 0;
-       }
-
-       /*
-        * We could walk the page table to read the non-linear
-        * pte, and get the page index without fetching the page
-        * from swap.  But that's a lot of code to duplicate here
-        * for a rare case, so we simply fetch the page.
-        */
-       err = get_user_pages(current, mm, address, 1, 0, 0, &page, NULL);
-       if (err >= 0) {
-               key->shared.pgoff =
-                       page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
-               put_page(page);
-               return 0;
+       } else {
+               key->both.offset |= FUT_OFF_INODE; /* inode-based key. */
+               key->shared.inode = page->mapping->inode;

Quoted 7 lines · Nick Piggin

Quoted 5 lines · Peter Zijlstra

On the way of getting rid of the mmap_sem requirement for shared futexes, start by relying on get_user_pages().

This requires we get the page associated with the key, and put the page when we're done with it.

Quoted 6 lines

Hi Peter,

Cool.

I'm all for removing mmap_sem requirement from shared futexes... Are there many apps which make a non-trivial use of them I wonder?

No idea. I've heard some stories, but I've never seen any code..

Quoted 2 lines

I guess it will help legacy (pre-FUTEX_PRIVATE) usespaces in performance too, though.

Yeah, that was one of the motivations for this patch.

Quoted 11 lines

What I'm worried about with this is invalidate or truncate races. With direct IO, it obviously doesn't matter because the only requirement is that the page existed at the address at some point during the syscall...

So I'd really like you to not carry the page around in the key, but just continue using the same key we have now. Also, lock the page and ensure it hasn't been truncated before taking the inode from the key and incrementing its count (page lock's extra atomics should be more or less cancelled out by fewer mmap_sem atomic ops).

Quoted 1 line

get_futex_key should look something like this I would have thought:?

Does look nicer, will have to ponder it a bit though.

I must admit to not fully understanding why we take inode/mm references in the key anyway as neither will stop unmapping the futex.

Will make this into a nice patch.. thanks!

Quoted 5 lines

BTW. I like that it removes a lot of fshared crap from around the place. And also this is a really good user of fast_gup because I guess it should usually be faulted in. The problem is that this could be a little more expensive for architectures that don't implement fast_gup. Though most should be able to.

Yeah, if that really becomes a problem (but I doubt it will) we could possibly make the old and new scheme work based on ARCH_HAVE_PTE_SPECIAL or something ugly like that.

kernel/futex.c +13 -17

@@ -191,7 +191,6 @@ static int get_futex_key(u32 __user *uad
 {
        unsigned long address = (unsigned long)uaddr;
        struct mm_struct *mm = current->mm;
-       struct vm_area_struct *vma;
        struct page *page;
        int err;
 

@@ -210,27 +209,26 @@ static int get_futex_key(u32 __user *uad
         * Note : We do have to check 'uaddr' is a valid user address,
         *        but access_ok() should be faster than find_vma()
         */
-       if (!fshared) {
+       if (likely(!fshared)) {
                if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
                        return -EFAULT;
                key->private.mm = mm;
                key->private.address = address;
                return 0;
        }
-       /*
-        * The futex is hashed differently depending on whether
-        * it's in a shared or private mapping.  So check vma first.
-        */
-       vma = find_extend_vma(mm, address);
-       if (unlikely(!vma))
-               return -EFAULT;
-
-       /*
-        * Permissions.
-        */
-       if (unlikely((vma->vm_flags & (VM_IO|VM_READ)) != VM_READ))
-               return (vma->vm_flags & VM_IO) ? -EPERM : -EACCES;
 
+again:
+       err = fast_gup(address, 1, 0, &page);
+       if (err < 0)
+               return err;
+
+       lock_page(page);
+       if (!page->mapping) { /* PageAnon pages shouldn't get caught here */
+               unlock_page(page);
+               put_page(page);
+               goto again;
+       }
+
        /*
         * Private mappings are handled in a simple way.
         *

@@ -240,38 +238,19 @@ static int get_futex_key(u32 __user *uad
         * VM_MAYSHARE here, not VM_SHARED which is restricted to shared
         * mappings of _writable_ handles.
         */
-       if (likely(!(vma->vm_flags & VM_MAYSHARE))) {
-               key->both.offset |= FUT_OFF_MMSHARED; /* reference taken on mm 
 *

/

kernel/futex.c +3 -26

+       if (PageAnon(page)) {
+               key->both.offset |= FUT_OFF_MMSHARED; /* ref taken on mm */
                key->private.mm = mm;
                key->private.address = address;
-               return 0;
-       }
-
-       /*
-        * Linear file mappings are also simple.
-        */
-       key->shared.inode = vma->vm_file->f_path.dentry->d_inode;
-       key->both.offset |= FUT_OFF_INODE; /* inode-based key. */
-       if (likely(!(vma->vm_flags & VM_NONLINEAR))) {
-               key->shared.pgoff = (((address - vma->vm_start) >> PAGE_SHIFT)
-                                    + vma->vm_pgoff);
-               return 0;
-       }
-
-       /*
-        * We could walk the page table to read the non-linear
-        * pte, and get the page index without fetching the page
-        * from swap.  But that's a lot of code to duplicate here
-        * for a rare case, so we simply fetch the page.
-        */
-       err = get_user_pages(current, mm, address, 1, 0, 0, &page, NULL);
-       if (err >= 0) {
-               key->shared.pgoff =
-                       page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
-               put_page(page);
-               return 0;
+       } else {

Hi,

this patch series removes mmap_sem from the fast path of shared futexes by making use of Nick's recent fast_gup() patches. Full series at:

http://programming.kicks-ass.net/kernel-patches/futex-fast_gup/v2.6.24.4-rt4/