lkml.org 
[lkml]   [2016]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] futex: Calculate the futex key based on a tail page for file-based futexes
On Tue, Jun 07, 2016 at 01:30:17PM +0100, Mel Gorman wrote:
> Mike Galbraith reported that the LTP test case futex_wake04 was broken
> by commit 65d8fc777f6d ("futex: Remove requirement for lock_page()
> in get_futex_key()").
>
> This test case uses futexes backed by hugetlbfs pages and so there is an
> associated inode with a futex stored on such pages. The problem is that
> the key is being calculated based on the head page index of the hugetlbfs
> page and not the tail page.
>
> Prior to the optimisation, the page lock was used to stabilise mappings and
> pin the inode is file-backed which is overkill. If the page was a compound
> page, the head page was automatically looked up as part of the page lock
> operation but the tail page index was used to calculate the futex key.
>
> After the optimisation, the compound head is looked up early and the page
> lock is only relied upon to identify truncated pages, special pages or a
> shmem page moving to swapcache. The head page is looked up because without
> the page lock, special care has to be taken to pin the inode correctly.
> However, the tail page is still required to calculate the futex key so this
> patch records the tail page.
>
> On vanilla 4.6, the output of the test case is;
>
> futex_wake04 0 TINFO : Hugepagesize 2097152
> futex_wake04 1 TFAIL : futex_wake04.c:126: Bug: wait_thread2 did not wake after 30 secs.
>
> With the patch applied
>
> futex_wake04 0 TINFO : Hugepagesize 2097152
> futex_wake04 1 TPASS : Hi hydra, thread2 awake!
>
> Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com>
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
> kernel/futex.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index c20f06f38ef3..6555d5459e98 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -469,7 +469,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
> {
> unsigned long address = (unsigned long)uaddr;
> struct mm_struct *mm = current->mm;
> - struct page *page;
> + struct page *page, *tail;
> struct address_space *mapping;
> int err, ro = 0;
>
> @@ -530,7 +530,15 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
> * considered here and page lock forces unnecessarily serialization
> * From this point on, mapping will be re-verified if necessary and
> * page lock will be acquired only if it is unavoidable
> - */
> + *
> + * Mapping checks require the head page for any compound page so the
> + * head page and mapping is looked up now. For anonymous pages, it
> + * does not matter if the page splits in the future as the key is
> + * based on the address. For filesystem-backed pages, the tail is
> + * required as the index of the page determines the key. For
> + * base pages, there is no tail page and tail == page.
> + */
> + tail = page;
> page = compound_head(page);
> mapping = READ_ONCE(page->mapping);
>
> @@ -654,7 +662,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
>
> key->both.offset |= FUT_OFF_INODE; /* inode-based key */
> key->shared.inode = inode;
> - key->shared.pgoff = basepage_index(page);
> + key->shared.pgoff = basepage_index(tail);
> rcu_read_unlock();
> }
>

\
 
 \ /
  Last update: 2016-06-07 20:01    [W:0.050 / U:0.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site