lkml.org 
[lkml]   [2012]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] thp: move release mmap_sem lock out of khugepaged_alloc_page
On 09/13/2012 06:18 AM, Andrew Morton wrote:
> On Wed, 12 Sep 2012 20:56:41 +0800
> Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote:
>
>> To make the code more clear, move release the lock out of khugepaged_alloc_page
>>
>> ...
>>
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -1854,11 +1854,6 @@ static struct page
>> *hpage = alloc_hugepage_vma(khugepaged_defrag(), vma, address,
>> node, __GFP_OTHER_NODE);
>>
>> - /*
>> - * After allocating the hugepage, release the mmap_sem read lock in
>> - * preparation for taking it in write mode.
>> - */
>> - up_read(&mm->mmap_sem);
>> if (unlikely(!*hpage)) {
>> count_vm_event(THP_COLLAPSE_ALLOC_FAILED);
>> *hpage = ERR_PTR(-ENOMEM);
>> @@ -1905,7 +1900,6 @@ static struct page
>> struct vm_area_struct *vma, unsigned long address,
>> int node)
>> {
>> - up_read(&mm->mmap_sem);
>> VM_BUG_ON(!*hpage);
>> return *hpage;
>> }
>> @@ -1931,8 +1925,14 @@ static void collapse_huge_page(struct mm_struct *mm,
>>
>> VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>>
>> - /* release the mmap_sem read lock. */
>> new_page = khugepaged_alloc_page(hpage, mm, vma, address, node);
>> +
>> + /*
>> + * After allocating the hugepage, release the mmap_sem read lock in
>> + * preparation for taking it in write mode.
>> + */
>> + up_read(&mm->mmap_sem);
>> +
>> if (!new_page)
>> return;
>
> Well that's a pretty minor improvement: one still has to go off on a
> big hunt to locate the matching down_read().
>
> And the patch will increase mmap_sem hold times by a teeny amount. Do
> we really want to do this?

Andrew,

This is why i did in the previous patch (the lock is released in alloc function),
but as you noticed, this is really teeny overload after this patch - only increase
the load of count_vm_event() which operates a cpu-local variable. And, before i
posted this patch, i did kerbench test, no regression was found.

The another approach is, let the function name indicate the lock will be released,
how about just change the function name to khugepaged_alloc_page_release_lock?








\
 
 \ /
  Last update: 2012-09-13 12:01    [W:0.038 / U:12.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site