[PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[jiang.biao2]

[-- Attachment #1: Type: text/plain, Size: 5381 bytes --]

From: Zhang Yi <zhang.yi20@zte.com.cn>

The futex-keys of processes share futex determined by page-offset, 
mapping-host, and 
mapping-index of the user space address.
User appications using hugepage for futex may lead to futex-key conflict. 
Assume there 
are two or more futexes in diffrent normal pages of the hugepage, and each 
futex has 
the same offset in its normal page, causing all the futexes have the same 
futex-key. 
In that case, futex may not work well.

This patch adds the normal page index in the compound page into the offset 
of futex-key.

Steps to reproduce the bug:
1. The 1st thread map a file of hugetlbfs, and use the return address as 
the 1st mutex's 
address, and use the return address with PAGE_SIZE added as the 2nd 
mutex's address;
2. The 1st thread initialize the two mutexes with pshared attribute, and 
lock the two mutexes.
3. The 1st thread create the 2nd thread, and the 2nd thread block on the 
1st mutex.
4. The 1st thread create the 3rd thread, and the 3rd thread block on the 
2nd mutex.
5. The 1st thread unlock the 2nd mutex, the 3rd thread can not take the 
2nd mutex, and 
may block forever.

Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn>
Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn>
Reviewed-by: Liu Dong <liu.dong3@zte.com.cn>
Reviewed-by: Cui Yunfeng <cui.yunfeng@zte.com.cn>
Reviewed-by: Lu Zhongjun <lu.zhongjun@zte.com.cn>
Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>

diff -uprN orig/linux-3.9-rc5/include/linux/mm.h 
new/linux-3.9-rc5/include/linux/mm.h
--- orig/linux-3.9-rc5/include/linux/mm.h       2013-03-31 
22:12:43.000000000 +0000
+++ new/linux-3.9-rc5/include/linux/mm.h        2013-04-03 
11:01:19.671403000 +0000
@@ -502,6 +502,20 @@ static inline void set_compound_order(st
        page[1].lru.prev = (void *)order;
 }
 
+static inline void set_page_compound_index(struct page *page, int index)
+{
+       if (PageHead(page))
+               return;
+       page->index = index;
+}
+
+static inline int get_page_compound_index(struct page *page)
+{
+       if (PageHead(page))
+               return 0;
+       return page->index;
+}
+
 #ifdef CONFIG_MMU
 /*
  * Do pte_mkwrite, but only if the vma says VM_WRITE.  We do this when
diff -uprN orig/linux-3.9-rc5/kernel/futex.c 
new/linux-3.9-rc5/kernel/futex.c
--- orig/linux-3.9-rc5/kernel/futex.c   2013-03-31 22:12:43.000000000 
+0000
+++ new/linux-3.9-rc5/kernel/futex.c    2013-04-03 11:03:42.168663000 
+0000
@@ -239,7 +239,7 @@ get_futex_key(u32 __user *uaddr, int fsh
        unsigned long address = (unsigned long)uaddr;
        struct mm_struct *mm = current->mm;
        struct page *page, *page_head;
-       int err, ro = 0;
+       int err, ro = 0, compound_index = 0;
 
        /*
         * The futex address must be "naturally" aligned.
@@ -299,6 +299,7 @@ again:
                         * freed from under us.
                         */
                        if (page != page_head) {
+                               compound_index = 
get_page_compound_index(page);
                                get_page(page_head);
                                put_page(page);
                        }
@@ -311,6 +312,7 @@ again:
 #else
        page_head = compound_head(page);
        if (page != page_head) {
+               compound_index = get_page_compound_index(page);
                get_page(page_head);
                put_page(page);
        }
@@ -363,7 +365,7 @@ again:
                key->private.mm = mm;
                key->private.address = address;
        } else {
-               key->both.offset |= FUT_OFF_INODE; /* inode-based key */
+               key->both.offset |= (compound_index << PAGE_SHIFT) | 
FUT_OFF_INODE; /* inode-based key */
                key->shared.inode = page_head->mapping->host;
                key->shared.pgoff = page_head->index;
        }
diff -uprN orig/linux-3.9-rc5/mm/hugetlb.c new/linux-3.9-rc5/mm/hugetlb.c
--- orig/linux-3.9-rc5/mm/hugetlb.c     2013-03-31 22:12:43.000000000 
+0000
+++ new/linux-3.9-rc5/mm/hugetlb.c      2013-04-03 11:02:10.556132000 
+0000
@@ -667,6 +667,7 @@ static void prep_compound_gigantic_page(
        for (i = 1; i < nr_pages; i++, p = mem_map_next(p, page, i)) {
                __SetPageTail(p);
                set_page_count(p, 0);
+               set_page_compound_index(p, i);
                p->first_page = page;
        }
 }
diff -uprN orig/linux-3.9-rc5/mm/page_alloc.c 
new/linux-3.9-rc5/mm/page_alloc.c
--- orig/linux-3.9-rc5/mm/page_alloc.c  2013-03-31 22:12:43.000000000 
+0000
+++ new/linux-3.9-rc5/mm/page_alloc.c   2013-04-03 11:01:47.933353000 
+0000
@@ -361,6 +361,7 @@ void prep_compound_page(struct page *pag
                struct page *p = page + i;
                __SetPageTail(p);
                set_page_count(p, 0);
+               set_page_compound_index(p, i);
                p->first_page = page;
        }
 }
--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s).  If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited.  If you have received this mail in error, please delete it and notify us immediately.

[-- Attachment #2: Type: text/html, Size: 12222 bytes --]

[PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[zhang.yi20]

Hello,

The futex-keys of processes share futex determined by page-offset, 
mapping-host, and 
mapping-index of the user space address. 
User appications using hugepage for futex may lead to futex-key conflict. 
Assume there 
are two or more futexes in diffrent normal pages of the hugepage, and each 
futex has 
the same offset in its normal page, causing all the futexes have the same 
futex-key. 
In that case, futex may not work well. 

This patch adds the normal page index in the compound page into the offset 
of futex-key. 

Steps to reproduce the bug: 
1. The 1st thread map a file of hugetlbfs, and use the return address as 
the 1st mutex's 
address, and use the return address with PAGE_SIZE added as the 2nd 
mutex's address; 
2. The 1st thread initialize the two mutexes with pshared attribute, and 
lock the two mutexes. 
3. The 1st thread create the 2nd thread, and the 2nd thread block on the 
1st mutex. 
4. The 1st thread create the 3rd thread, and the 3rd thread block on the 
2nd mutex. 
5. The 1st thread unlock the 2nd mutex, the 3rd thread can not take the 
2nd mutex, and 
may block forever. 

Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn>
Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn>
Reviewed-by: Liu Dong <liu.dong3@zte.com.cn>
Reviewed-by: Cui Yunfeng <cui.yunfeng@zte.com.cn>
Reviewed-by: Lu Zhongjun <lu.zhongjun@zte.com.cn>
Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>

diff -uprN orig/linux-3.9-rc7/include/linux/mm.h 
new/linux-3.9-rc7/include/linux/mm.h
--- orig/linux-3.9-rc7/include/linux/mm.h       2013-04-15 
00:45:16.000000000 +0000
+++ new/linux-3.9-rc7/include/linux/mm.h        2013-04-16 
11:21:59.573458000 +0000
@@ -502,6 +502,20 @@ static inline void set_compound_order(st
        page[1].lru.prev = (void *)order;
 }
 
+static inline void set_page_compound_index(struct page *page, int index)
+{
+       if (PageHead(page))
+               return;
+       page->index = index;
+}
+
+static inline int get_page_compound_index(struct page *page)
+{
+       if (PageHead(page))
+               return 0;
+       return page->index;
+}
+
 #ifdef CONFIG_MMU
 /*
  * Do pte_mkwrite, but only if the vma says VM_WRITE.  We do this when
diff -uprN orig/linux-3.9-rc7/kernel/futex.c 
new/linux-3.9-rc7/kernel/futex.c
--- orig/linux-3.9-rc7/kernel/futex.c   2013-04-15 00:45:16.000000000 
+0000
+++ new/linux-3.9-rc7/kernel/futex.c    2013-04-16 11:13:30.069887000 
+0000
@@ -239,7 +239,7 @@ get_futex_key(u32 __user *uaddr, int fsh
        unsigned long address = (unsigned long)uaddr;
        struct mm_struct *mm = current->mm;
        struct page *page, *page_head;
-       int err, ro = 0;
+       int err, ro = 0, comp_idx = 0;
 
        /*
         * The futex address must be "naturally" aligned.
@@ -299,6 +299,7 @@ again:
                         * freed from under us.
                         */
                        if (page != page_head) {
+                               comp_idx = get_page_compound_index(page);
                                get_page(page_head);
                                put_page(page);
                        }
@@ -311,6 +312,7 @@ again:
 #else
        page_head = compound_head(page);
        if (page != page_head) {
+               comp_idx = get_page_compound_index(page);
                get_page(page_head);
                put_page(page);
        }
@@ -363,7 +365,8 @@ again:
                key->private.mm = mm;
                key->private.address = address;
        } else {
-               key->both.offset |= FUT_OFF_INODE; /* inode-based key */
+               key->both.offset |= (comp_idx << PAGE_SHIFT)
+                                   | FUT_OFF_INODE; /* inode-based key */
                key->shared.inode = page_head->mapping->host;
                key->shared.pgoff = page_head->index;
        }
diff -uprN orig/linux-3.9-rc7/mm/hugetlb.c new/linux-3.9-rc7/mm/hugetlb.c
--- orig/linux-3.9-rc7/mm/hugetlb.c     2013-04-15 00:45:16.000000000 
+0000
+++ new/linux-3.9-rc7/mm/hugetlb.c      2013-04-16 10:23:02.658531000 
+0000
@@ -667,6 +667,7 @@ static void prep_compound_gigantic_page(
        for (i = 1; i < nr_pages; i++, p = mem_map_next(p, page, i)) {
                __SetPageTail(p);
                set_page_count(p, 0);
+               set_page_compound_index(p, i);
                p->first_page = page;
        }
 }
diff -uprN orig/linux-3.9-rc7/mm/page_alloc.c 
new/linux-3.9-rc7/mm/page_alloc.c
--- orig/linux-3.9-rc7/mm/page_alloc.c  2013-04-15 00:45:16.000000000 
+0000
+++ new/linux-3.9-rc7/mm/page_alloc.c   2013-04-16 10:23:16.452393000 
+0000
@@ -361,6 +361,7 @@ void prep_compound_page(struct page *pag
                struct page *p = page + i;
                __SetPageTail(p);
                set_page_count(p, 0);
+               set_page_compound_index(p, i);
                p->first_page = page;
        }
 }


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/15/2013 08:37 PM, zhang.yi20@zte.com.cn wrote:
> Hello,
>

Hi Zhang,

I've rewrapped your plain text here for legibility, please adjust your
mail client accordingly.

> The futex-keys of processes share futex determined by page-offset,
> mapping-host, and mapping-index of the user space address.  User appications
> using hugepage for futex may lead to futex-key conflict.  Assume there are two
> or more futexes in diffrent normal pages of the hugepage, and each futex has
> the same offset in its normal page, causing all the futexes have the same
> futex-key.  In that case, futex may not work well.
>
> This patch adds the normal page index in the compound page into the offset of
> futex-key.

It also modifies the mm prep_compound*page() routines to set the page
compound index. You didn't modify the structure itself, I'm curious why
this information wasn't set before? Something for the MM folks I guess..

>
> Steps to reproduce the bug:
> 1. The 1st thread map a file of hugetlbfs, and use the return address as the
>    1st mutex's address, and use the return address with PAGE_SIZE added as the
>    2nd mutex's address;
> 2. The 1st thread initialize the two mutexes with pshared attribute, and lock
>    the two mutexes.
> 3. The 1st thread create the 2nd thread, and the 2nd thread block on the 1st
>    mutex.
> 4. The 1st thread create the 3rd thread, and the 3rd thread block on the 2nd
>    mutex.
> 5. The 1st thread unlock the 2nd mutex, the 3rd thread can not take the 2nd
>    mutex, and may block forever.


Again, a functional testcase in futextest would be a good idea. This
helps validate the patch and also can be used to identify regressions in
the future.


> Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn>
> Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn>
> Reviewed-by: Liu Dong <liu.dong3@zte.com.cn>
> Reviewed-by: Cui Yunfeng <cui.yunfeng@zte.com.cn>
> Reviewed-by: Lu Zhongjun <lu.zhongjun@zte.com.cn>
> Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>
>
> diff -uprN orig/linux-3.9-rc7/include/linux/mm.h
> new/linux-3.9-rc7/include/linux/mm.h
> --- orig/linux-3.9-rc7/include/linux/mm.h       2013-04-15
> 00:45:16.000000000 +0000
> +++ new/linux-3.9-rc7/include/linux/mm.h        2013-04-16
> 11:21:59.573458000 +0000
> @@ -502,6 +502,20 @@ static inline void set_compound_order(st
>         page[1].lru.prev = (void *)order;
>  }
>
> +static inline void set_page_compound_index(struct page *page, int index)
> +{
> +       if (PageHead(page))
> +               return;
> +       page->index = index;
> +}

I presume the spaces instead of tabs is a result of your mailer mangling
whitespace?

> +
> +static inline int get_page_compound_index(struct page *page)
> +{
> +       if (PageHead(page))
> +               return 0;
> +       return page->index;
> +}
> +
>  #ifdef CONFIG_MMU
>  /*
>   * Do pte_mkwrite, but only if the vma says VM_WRITE.  We do this when
> diff -uprN orig/linux-3.9-rc7/kernel/futex.c
> new/linux-3.9-rc7/kernel/futex.c
> --- orig/linux-3.9-rc7/kernel/futex.c   2013-04-15 00:45:16.000000000
> +0000
> +++ new/linux-3.9-rc7/kernel/futex.c    2013-04-16 11:13:30.069887000
> +0000
> @@ -239,7 +239,7 @@ get_futex_key(u32 __user *uaddr, int fsh
>         unsigned long address = (unsigned long)uaddr;
>         struct mm_struct *mm = current->mm;
>         struct page *page, *page_head;
> -       int err, ro = 0;
> +       int err, ro = 0, comp_idx = 0;
>
>         /*
>          * The futex address must be "naturally" aligned.
> @@ -299,6 +299,7 @@ again:
>                          * freed from under us.
>                          */
>                         if (page != page_head) {
> +                               comp_idx = get_page_compound_index(page);
>                                 get_page(page_head);
>                                 put_page(page);
>                         }
> @@ -311,6 +312,7 @@ again:
>  #else
>         page_head = compound_head(page);
>         if (page != page_head) {
> +               comp_idx = get_page_compound_index(page);
>                 get_page(page_head);
>                 put_page(page);
>         }
> @@ -363,7 +365,8 @@ again:
>                 key->private.mm = mm;
>                 key->private.address = address;
>         } else {
> -               key->both.offset |= FUT_OFF_INODE; /* inode-based key */
> +               key->both.offset |= (comp_idx << PAGE_SHIFT)
> +                                   | FUT_OFF_INODE; /* inode-based key */

Comments at the end of lines are bad form already, when moving to
multi-line, please move the comment just above the statements.

What is the max value of comp_idx? Are we at risk of truncating it?
Looks like not really from my initial look.

This also needs a comment in futex.h describing the usage of the offset
field in union futex_key as well as above get_futex_key describing the
key for shared mappings.


Thanks,

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Dave Hansen]

Instead of bothering to store the index, why not just calculate it, like:

On 04/15/2013 08:37 PM, zhang.yi20@zte.com.cn wrote:
> +static inline int get_page_compound_index(struct page *page)
> +{
> +       if (PageHead(page))
> +               return 0;
> +       return compound_head(page) - page;
> +}

BTW, you've really got to get your mail client fixed.  Your patch is
still line-wrapped.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/16/2013 11:37 AM, Dave Hansen wrote:
> Instead of bothering to store the index, why not just calculate it, like:
> 
> On 04/15/2013 08:37 PM, zhang.yi20@zte.com.cn wrote:
>> +static inline int get_page_compound_index(struct page *page)
>> +{
>> +       if (PageHead(page))
>> +               return 0;
>> +       return compound_head(page) - page;
>> +}
> 
> BTW, you've really got to get your mail client fixed.  Your patch is
> still line-wrapped.

And with this it would no longer be necessary to store this index at
all, eliminating all changes to the MM other than this accessor function
- which if not needed there could be added to futex.c, or even replaced with
"page_head - page" in get_futex_key() right?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[zhang.yi20]

Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/17 01:57:10:

> Again, a functional testcase in futextest would be a good idea. This
> helps validate the patch and also can be used to identify regressions in
> the future.

I will post the testcase code later.

> 
> What is the max value of comp_idx? Are we at risk of truncating it?
> Looks like not really from my initial look.
> 
> This also needs a comment in futex.h describing the usage of the offset
> field in union futex_key as well as above get_futex_key describing the
> key for shared mappings.
> 
> 

As far as I know , the max size of one hugepage is 1 GBytes for x86 cpu.
Can some other cpus support greater hugepage even more than 4 GBytes? If 
so, we can change the type of 'offset' from int to long to avoid 
truncating.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/17/2013 02:55 AM, zhang.yi20@zte.com.cn wrote:
> Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/17 01:57:10:
> 
>> Again, a functional testcase in futextest would be a good idea. This
>> helps validate the patch and also can be used to identify regressions in
>> the future.
> 
> I will post the testcase code later.
> 
>>
>> What is the max value of comp_idx? Are we at risk of truncating it?
>> Looks like not really from my initial look.
>>
>> This also needs a comment in futex.h describing the usage of the offset
>> field in union futex_key as well as above get_futex_key describing the
>> key for shared mappings.
>>
>>
> 
> As far as I know , the max size of one hugepage is 1 GBytes for x86 cpu.
> Can some other cpus support greater hugepage even more than 4 GBytes? If 
> so, we can change the type of 'offset' from int to long to avoid 
> truncating.

I discussed this with Dave Hansen, on CC, and he thought we needed 9
bits, so even on x86 32b we should be covered.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Dave Hansen]

On 04/17/2013 07:18 AM, Darren Hart wrote:
>>> This also needs a comment in futex.h describing the usage of the offset
>>> field in union futex_key as well as above get_futex_key describing the
>>> key for shared mappings.
>>>
>> As far as I know , the max size of one hugepage is 1 GBytes for x86 cpu.
>> Can some other cpus support greater hugepage even more than 4 GBytes? If 
>> so, we can change the type of 'offset' from int to long to avoid 
>> truncating.
> 
> I discussed this with Dave Hansen, on CC, and he thought we needed 9
> bits, so even on x86 32b we should be covered.

I think the problem is actually on 64-bit since you still only have
32-bits in an 'int' there.

I guess it's remotely possible that we could have some
mega-super-huge-gigantic pages show up in hardware some day, or that
somebody would come up with software-only one.  I bet there's a lot more
code that will break in the kernel than this futex code, though.

The other option would be to start #defining some build-time constant
for what the largest possible huge page size is, then BUILD_BUG_ON() it.

Or you can just make it a long ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/17/2013 08:26 AM, Dave Hansen wrote:
> On 04/17/2013 07:18 AM, Darren Hart wrote:
>>>> This also needs a comment in futex.h describing the usage of the offset
>>>> field in union futex_key as well as above get_futex_key describing the
>>>> key for shared mappings.
>>>>
>>> As far as I know , the max size of one hugepage is 1 GBytes for x86 cpu.
>>> Can some other cpus support greater hugepage even more than 4 GBytes? If 
>>> so, we can change the type of 'offset' from int to long to avoid 
>>> truncating.
>>
>> I discussed this with Dave Hansen, on CC, and he thought we needed 9
>> bits, so even on x86 32b we should be covered.
> 
> I think the problem is actually on 64-bit since you still only have
> 32-bits in an 'int' there.
> 
> I guess it's remotely possible that we could have some
> mega-super-huge-gigantic pages show up in hardware some day, or that
> somebody would come up with software-only one.  I bet there's a lot more
> code that will break in the kernel than this futex code, though.
> 
> The other option would be to start #defining some build-time constant
> for what the largest possible huge page size is, then BUILD_BUG_ON() it.
> 
> Or you can just make it a long ;)

If we make it a long I'd want to see futextest performance tests before
and after. Messing with the futex_key has been known to have bad results
in the past :-)

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[zhang.yi20]

Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/17 23:51:36:

> On 04/17/2013 08:26 AM, Dave Hansen wrote:
> > On 04/17/2013 07:18 AM, Darren Hart wrote:
> >>>> This also needs a comment in futex.h describing the usage of the
> >>>> offset field in union futex_key as well as above get_futex_key
> >>>> describing the key for shared mappings.
> >>>>
> >>> As far as I know , the max size of one hugepage is 1 GBytes for 
> >>> x86 cpu. Can some other cpus support greater hugepage even more 
> >>> than 4 GBytes? If so, we can change the type of 'offset' from int 
> >>>  to long to avoid truncating.
> >>
> >> I discussed this with Dave Hansen, on CC, and he thought we needed
> >> 9 bits, so even on x86 32b we should be covered.
> > 
> > I think the problem is actually on 64-bit since you still only have
> > 32-bits in an 'int' there.
> > 
> > I guess it's remotely possible that we could have some
> > mega-super-huge-gigantic pages show up in hardware some day, or that
> > somebody would come up with software-only one.  I bet there's a lot
> > more code that will break in the kernel than this futex code, though.
> > 
> > The other option would be to start #defining some build-time constant
> > for what the largest possible huge page size is, then BUILD_BUG_ON()
> > it.
> >
> > Or you can just make it a long ;)
> 
> If we make it a long I'd want to see futextest performance tests before
> and after. Messing with the futex_key has been known to have bad results
> in the past :-)
> 
> -- 
 
I have run futextest/performance/futex_wait for testing, 5 times before 
make it long:
futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10215 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 9862 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10081 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10060 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10081 Kiter/s


And 5 times after make it long:
futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 9940 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10204 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 9901 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10152 Kiter/s

futex_wait: Measure FUTEX_WAIT operations per second
        Arguments: iterations=100000000 threads=256
Result: 10060 Kiter/s


Seems OK, is it?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/18/2013 01:05 AM, zhang.yi20@zte.com.cn wrote:
> Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/17 23:51:36:
> 
>> On 04/17/2013 08:26 AM, Dave Hansen wrote:
>>> On 04/17/2013 07:18 AM, Darren Hart wrote:
>>>>>> This also needs a comment in futex.h describing the usage of the
>>>>>> offset field in union futex_key as well as above get_futex_key
>>>>>> describing the key for shared mappings.
>>>>>>
>>>>> As far as I know , the max size of one hugepage is 1 GBytes for 
>>>>> x86 cpu. Can some other cpus support greater hugepage even more 
>>>>> than 4 GBytes? If so, we can change the type of 'offset' from int 
>>>>>  to long to avoid truncating.
>>>>
>>>> I discussed this with Dave Hansen, on CC, and he thought we needed
>>>> 9 bits, so even on x86 32b we should be covered.
>>>
>>> I think the problem is actually on 64-bit since you still only have
>>> 32-bits in an 'int' there.
>>>
>>> I guess it's remotely possible that we could have some
>>> mega-super-huge-gigantic pages show up in hardware some day, or that
>>> somebody would come up with software-only one.  I bet there's a lot
>>> more code that will break in the kernel than this futex code, though.
>>>
>>> The other option would be to start #defining some build-time constant
>>> for what the largest possible huge page size is, then BUILD_BUG_ON()
>>> it.
>>>
>>> Or you can just make it a long ;)
>>
>> If we make it a long I'd want to see futextest performance tests before
>> and after. Messing with the futex_key has been known to have bad results
>> in the past :-)
>>
>> -- 
>  
> I have run futextest/performance/futex_wait for testing, 5 times before 
> make it long:
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10215 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 9862 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10081 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10060 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10081 Kiter/s
> 
> 
> And 5 times after make it long:
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 9940 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10204 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 9901 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10152 Kiter/s
> 
> futex_wait: Measure FUTEX_WAIT operations per second
>         Arguments: iterations=100000000 threads=256
> Result: 10060 Kiter/s
> 
> 
> Seems OK, is it?
> 

Changes appear to be in the noise, no impact with this load anyway.

How many CPUs on your test machine? I presume not 256?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[zhang.yi20]

Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/18 22:34:29:

> On 04/18/2013 01:05 AM, zhang.yi20@zte.com.cn wrote:
> > 
> > I have run futextest/performance/futex_wait for testing, 
> >  5 times before make it long:
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10215 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 9862 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10081 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10060 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10081 Kiter/s
> > 
> > 
> > And 5 times after make it long:
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 9940 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10204 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 9901 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10152 Kiter/s
> > 
> > futex_wait: Measure FUTEX_WAIT operations per second
> >         Arguments: iterations=100000000 threads=256
> > Result: 10060 Kiter/s
> > 
> > 
> > Seems OK, is it?
> > 
> 
> Changes appear to be in the noise, no impact with this load 
> anyway.
> How many CPUs on your test machine? I presume not 256?
> 
> -- 

There are 16 CPUs， and mode is:
Intel(R) Xeon(R) CPU           C5528  @ 2.13GHz

Shall I make the number of threads as the CPUS? I test again with argument 
'-n 16', the result is similar.

BTW, have you seen the testcase in my other mail?  It seems to be rejected 
by LKML.

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

On 04/18/2013 07:13 PM, zhang.yi20@zte.com.cn wrote:
> Darren Hart <dvhart@linux.intel.com> wrote on 2013/04/18 22:34:29:
> 
>> On 04/18/2013 01:05 AM, zhang.yi20@zte.com.cn wrote:
>>>
>>> I have run futextest/performance/futex_wait for testing, 
>>>  5 times before make it long:
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10215 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 9862 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10081 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10060 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10081 Kiter/s
>>>
>>>
>>> And 5 times after make it long:
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 9940 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10204 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 9901 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10152 Kiter/s
>>>
>>> futex_wait: Measure FUTEX_WAIT operations per second
>>>         Arguments: iterations=100000000 threads=256
>>> Result: 10060 Kiter/s
>>>
>>>
>>> Seems OK, is it?
>>>
>>
>> Changes appear to be in the noise, no impact with this load 
>> anyway.
>> How many CPUs on your test machine? I presume not 256?
>>
>> -- 
> 
> There are 16 CPUsGBP! and mode is:
> Intel(R) Xeon(R) CPU           C5528  @ 2.13GHz
> 
> Shall I make the number of threads as the CPUS? I test again with argument 
> '-n 16', the result is similar.

No, I just wanted to be sure you weren't running 256 threads on 1 CPU as
you wouldn't be likely to be stressing the bucket list much :-)

> BTW, have you seen the testcase in my other mail?  It seems to be rejected 
> by LKML.

Might have something to do with what appears to still be HTML email. You
really need to fix your email client.

See:  http://www.tux.org/lkml/
#12 in particular.


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] futex: bugfix for futex-key conflict when futex use hugepage

[Darren Hart]

> 
> BTW, have you seen the testcase in my other mail?  It seems to be rejected 
> by LKML.
> 

I did not receive it, did you also CC me?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>