linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hugetlb: call mmu notifiers on hugepage cow
@ 2010-05-27 20:43 Doug Doan
  2010-05-28  9:59 ` Mel Gorman
  2010-06-02  6:16 ` Andrew Morton
  0 siblings, 2 replies; 10+ messages in thread
From: Doug Doan @ 2010-05-27 20:43 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, andi, lee.schermerhorn, rientjes, mel, akpm

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

From: Doug Doan <dougd@cray.com>

When a copy-on-write occurs, we take one of two paths in handle_mm_fault: 
through handle_pte_fault for normal pages, or through hugetlb_fault for huge pages.

In the normal page case, we eventually get to do_wp_page and call mmu notifiers 
via ptep_clear_flush_notify. There is no callout to the mmmu notifiers in the 
huge page case. This patch fixes that.

Signed-off-by: Doug Doan <dougd@cray.com>
---

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 801 bytes --]

--- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
+++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
@@ -2345,11 +2345,17 @@ retry_avoidcopy:
 	ptep = huge_pte_offset(mm, address & huge_page_mask(h));
 	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
 		/* Break COW */
+		mmu_notifier_invalidate_range_start(mm,
+			address & huge_page_mask(h),
+			(address & huge_page_mask(h)) + huge_page_size(h));
 		huge_ptep_clear_flush(vma, address, ptep);
 		set_huge_pte_at(mm, address, ptep,
 				make_huge_pte(vma, new_page, 1));
 		/* Make the old page be freed below */
 		new_page = old_page;
+		mmu_notifier_invalidate_range_end(mm,
+			address & huge_page_mask(h),
+			(address & huge_page_mask(h)) + huge_page_size(h));
 	}
 	page_cache_release(new_page);
 	page_cache_release(old_page);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-05-27 20:43 [PATCH] hugetlb: call mmu notifiers on hugepage cow Doug Doan
@ 2010-05-28  9:59 ` Mel Gorman
  2010-05-28 16:39   ` Doug Doan
  2010-06-02  6:16 ` Andrew Morton
  1 sibling, 1 reply; 10+ messages in thread
From: Mel Gorman @ 2010-05-28  9:59 UTC (permalink / raw)
  To: Doug Doan; +Cc: linux-mm, linux-kernel, andi, lee.schermerhorn, rientjes, akpm

On Thu, May 27, 2010 at 01:43:00PM -0700, Doug Doan wrote:
> From: Doug Doan <dougd@cray.com>
>
> When a copy-on-write occurs, we take one of two paths in handle_mm_fault: 
> through handle_pte_fault for normal pages, or through hugetlb_fault for 
> huge pages.
>
> In the normal page case, we eventually get to do_wp_page and call mmu 
> notifiers via ptep_clear_flush_notify. There is no callout to the mmmu 
> notifiers in the huge page case. This patch fixes that.
>
> Signed-off-by: Doug Doan <dougd@cray.com>
> ---

> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>  	ptep = huge_pte_offset(mm, address & huge_page_mask(h));
>  	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>  		/* Break COW */
> +		mmu_notifier_invalidate_range_start(mm,
> +			address & huge_page_mask(h),
> +			(address & huge_page_mask(h)) + huge_page_size(h));

Should the address not already be aligned?

Otherwise, I don't see any problem.

>  		huge_ptep_clear_flush(vma, address, ptep);
>  		set_huge_pte_at(mm, address, ptep,
>  				make_huge_pte(vma, new_page, 1));
>  		/* Make the old page be freed below */
>  		new_page = old_page;
> +		mmu_notifier_invalidate_range_end(mm,
> +			address & huge_page_mask(h),
> +			(address & huge_page_mask(h)) + huge_page_size(h));
>  	}
>  	page_cache_release(new_page);
>  	page_cache_release(old_page);


-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-05-28  9:59 ` Mel Gorman
@ 2010-05-28 16:39   ` Doug Doan
  2010-05-28 17:34     ` Mel Gorman
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Doan @ 2010-05-28 16:39 UTC (permalink / raw)
  To: Mel Gorman
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	akpm@linux-foundation.org

On 05/28/2010 02:59 AM, Mel Gorman wrote:
> On Thu, May 27, 2010 at 01:43:00PM -0700, Doug Doan wrote:
>> From: Doug Doan<dougd@cray.com>
>>
>> When a copy-on-write occurs, we take one of two paths in handle_mm_fault:
>> through handle_pte_fault for normal pages, or through hugetlb_fault for
>> huge pages.
>>
>> In the normal page case, we eventually get to do_wp_page and call mmu
>> notifiers via ptep_clear_flush_notify. There is no callout to the mmmu
>> notifiers in the huge page case. This patch fixes that.
>>
>> Signed-off-by: Doug Doan<dougd@cray.com>
>> ---
>
>> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
>> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
>> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>>   	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
>>   	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>>   		/* Break COW */
>> +		mmu_notifier_invalidate_range_start(mm,
>> +			address&  huge_page_mask(h),
>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>
> Should the address not already be aligned?

I'm not seeing where the address was aligned before this point. The code just 
above aligns it:

    	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
    	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
    		/* Break COW */

> Otherwise, I don't see any problem.
>
>>   		huge_ptep_clear_flush(vma, address, ptep);
>>   		set_huge_pte_at(mm, address, ptep,
>>   				make_huge_pte(vma, new_page, 1));
>>   		/* Make the old page be freed below */
>>   		new_page = old_page;
>> +		mmu_notifier_invalidate_range_end(mm,
>> +			address&  huge_page_mask(h),
>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>>   	}
>>   	page_cache_release(new_page);
>>   	page_cache_release(old_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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-05-28 16:39   ` Doug Doan
@ 2010-05-28 17:34     ` Mel Gorman
  0 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2010-05-28 17:34 UTC (permalink / raw)
  To: Doug Doan
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	akpm@linux-foundation.org

On Fri, May 28, 2010 at 09:39:59AM -0700, Doug Doan wrote:
> On 05/28/2010 02:59 AM, Mel Gorman wrote:
>> On Thu, May 27, 2010 at 01:43:00PM -0700, Doug Doan wrote:
>>> From: Doug Doan<dougd@cray.com>
>>>
>>> When a copy-on-write occurs, we take one of two paths in handle_mm_fault:
>>> through handle_pte_fault for normal pages, or through hugetlb_fault for
>>> huge pages.
>>>
>>> In the normal page case, we eventually get to do_wp_page and call mmu
>>> notifiers via ptep_clear_flush_notify. There is no callout to the mmmu
>>> notifiers in the huge page case. This patch fixes that.
>>>
>>> Signed-off-by: Doug Doan<dougd@cray.com>
>>> ---
>>
>>> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
>>> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
>>> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>>>   	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
>>>   	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>>>   		/* Break COW */
>>> +		mmu_notifier_invalidate_range_start(mm,
>>> +			address&  huge_page_mask(h),
>>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>>
>> Should the address not already be aligned?
>
> I'm not seeing where the address was aligned before this point. The code 
> just above aligns it:
>
>    	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
>    	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>    		/* Break COW */
>

I'm sorry. You're right. I was looking at the copy_huge_page which was
not aligning the address. It should be but it's ultimately harmless as
the parameter is discarded.

Acked-by: Mel Gorman <mel@csn.ul.ie>

>> Otherwise, I don't see any problem.
>>
>>>   		huge_ptep_clear_flush(vma, address, ptep);
>>>   		set_huge_pte_at(mm, address, ptep,
>>>   				make_huge_pte(vma, new_page, 1));
>>>   		/* Make the old page be freed below */
>>>   		new_page = old_page;
>>> +		mmu_notifier_invalidate_range_end(mm,
>>> +			address&  huge_page_mask(h),
>>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>>>   	}
>>>   	page_cache_release(new_page);
>>>   	page_cache_release(old_page);
>>
>>
>

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-05-27 20:43 [PATCH] hugetlb: call mmu notifiers on hugepage cow Doug Doan
  2010-05-28  9:59 ` Mel Gorman
@ 2010-06-02  6:16 ` Andrew Morton
  2010-06-02 23:13   ` Doug Doan
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2010-06-02  6:16 UTC (permalink / raw)
  To: Doug Doan
  Cc: linux-mm, linux-kernel, andi, lee.schermerhorn, rientjes, mel,
	Andrea Arcangeli

On Thu, 27 May 2010 13:43:00 -0700 Doug Doan <dougd@cray.com> wrote:

> 
> When a copy-on-write occurs, we take one of two paths in handle_mm_fault: 
> through handle_pte_fault for normal pages, or through hugetlb_fault for huge pages.
> 
> In the normal page case, we eventually get to do_wp_page and call mmu notifiers 
> via ptep_clear_flush_notify. There is no callout to the mmmu notifiers in the 
> huge page case. This patch fixes that.
> 
> Signed-off-by: Doug Doan <dougd@cray.com>
> ---
> 
> [patch  text/plain (802B)]
> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700

(In patch -p1 form, please.  So a/mm/hugetlb.c)

> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>  	ptep = huge_pte_offset(mm, address & huge_page_mask(h));
>  	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>  		/* Break COW */
> +		mmu_notifier_invalidate_range_start(mm,
> +			address & huge_page_mask(h),
> +			(address & huge_page_mask(h)) + huge_page_size(h));
>  		huge_ptep_clear_flush(vma, address, ptep);
>  		set_huge_pte_at(mm, address, ptep,
>  				make_huge_pte(vma, new_page, 1));
>  		/* Make the old page be freed below */
>  		new_page = old_page;
> +		mmu_notifier_invalidate_range_end(mm,
> +			address & huge_page_mask(h),
> +			(address & huge_page_mask(h)) + huge_page_size(h));
>  	}
>  	page_cache_release(new_page);
>  	page_cache_release(old_page);

This causes mmu_notifier_invalidate_range_start() to be called under
page_table_lock.  The immediately preceding code seems to take some
care to avoid doing that.  I took a quick look at other callsites and
cannot immediately see other cases where
mmu_notifier_invalidate_range_start/end() are called under that lock.

This may not introduce bugs with current notifier implementations (I
didn't check), but it does lessen flexibility?

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-06-02  6:16 ` Andrew Morton
@ 2010-06-02 23:13   ` Doug Doan
  2010-06-02 23:33     ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Doan @ 2010-06-02 23:13 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	mel@csn.ul.ie, Andrea Arcangeli

On 06/01/2010 11:16 PM, Andrew Morton wrote:
> On Thu, 27 May 2010 13:43:00 -0700 Doug Doan<dougd@cray.com>  wrote:
>
>>
>> When a copy-on-write occurs, we take one of two paths in handle_mm_fault:
>> through handle_pte_fault for normal pages, or through hugetlb_fault for huge pages.
>>
>> In the normal page case, we eventually get to do_wp_page and call mmu notifiers
>> via ptep_clear_flush_notify. There is no callout to the mmmu notifiers in the
>> huge page case. This patch fixes that.
>>
>> Signed-off-by: Doug Doan<dougd@cray.com>
>> ---
>>
>> [patch  text/plain (802B)]
>> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
>> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
>
> (In patch -p1 form, please.  So a/mm/hugetlb.c)
>
>> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>>   	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
>>   	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>>   		/* Break COW */
>> +		mmu_notifier_invalidate_range_start(mm,
>> +			address&  huge_page_mask(h),
>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>>   		huge_ptep_clear_flush(vma, address, ptep);
>>   		set_huge_pte_at(mm, address, ptep,
>>   				make_huge_pte(vma, new_page, 1));
>>   		/* Make the old page be freed below */
>>   		new_page = old_page;
>> +		mmu_notifier_invalidate_range_end(mm,
>> +			address&  huge_page_mask(h),
>> +			(address&  huge_page_mask(h)) + huge_page_size(h));
>>   	}
>>   	page_cache_release(new_page);
>>   	page_cache_release(old_page);
>
> This causes mmu_notifier_invalidate_range_start() to be called under
> page_table_lock.  The immediately preceding code seems to take some
> care to avoid doing that.  I took a quick look at other callsites and
> cannot immediately see other cases where
> mmu_notifier_invalidate_range_start/end() are called under that lock.
>
> This may not introduce bugs with current notifier implementations (I
> didn't check), but it does lessen flexibility?

In the normal page case, handle_pte_fault calls do_wp_page inside a spinlock on 
ptl = pte_lockptr(mm, pmd), which uses mm->page_table_lock if USE_SPLIT_PTLOCKS 
is not defined.

I don't understand what you mean by lessen flexibilty.

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-06-02 23:13   ` Doug Doan
@ 2010-06-02 23:33     ` Andrew Morton
  2010-06-03 17:36       ` Doug Doan
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2010-06-02 23:33 UTC (permalink / raw)
  To: Doug Doan
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	mel@csn.ul.ie, Andrea Arcangeli

On Wed, 2 Jun 2010 16:13:42 -0700
Doug Doan <dougd@cray.com> wrote:

> On 06/01/2010 11:16 PM, Andrew Morton wrote:
> > On Thu, 27 May 2010 13:43:00 -0700 Doug Doan<dougd@cray.com>  wrote:
> >
> >>
> >> When a copy-on-write occurs, we take one of two paths in handle_mm_fault:
> >> through handle_pte_fault for normal pages, or through hugetlb_fault for huge pages.
> >>
> >> In the normal page case, we eventually get to do_wp_page and call mmu notifiers
> >> via ptep_clear_flush_notify. There is no callout to the mmmu notifiers in the
> >> huge page case. This patch fixes that.
> >>
> >> Signed-off-by: Doug Doan<dougd@cray.com>
> >> ---
> >>
> >> [patch  text/plain (802B)]
> >> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
> >> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
> >
> > (In patch -p1 form, please.  So a/mm/hugetlb.c)
> >
> >> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
> >>   	ptep = huge_pte_offset(mm, address&  huge_page_mask(h));
> >>   	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
> >>   		/* Break COW */
> >> +		mmu_notifier_invalidate_range_start(mm,
> >> +			address&  huge_page_mask(h),
> >> +			(address&  huge_page_mask(h)) + huge_page_size(h));
> >>   		huge_ptep_clear_flush(vma, address, ptep);
> >>   		set_huge_pte_at(mm, address, ptep,
> >>   				make_huge_pte(vma, new_page, 1));
> >>   		/* Make the old page be freed below */
> >>   		new_page = old_page;
> >> +		mmu_notifier_invalidate_range_end(mm,
> >> +			address&  huge_page_mask(h),
> >> +			(address&  huge_page_mask(h)) + huge_page_size(h));
> >>   	}
> >>   	page_cache_release(new_page);
> >>   	page_cache_release(old_page);
> >
> > This causes mmu_notifier_invalidate_range_start() to be called under
> > page_table_lock.  The immediately preceding code seems to take some
> > care to avoid doing that.  I took a quick look at other callsites and
> > cannot immediately see other cases where
> > mmu_notifier_invalidate_range_start/end() are called under that lock.
> >
> > This may not introduce bugs with current notifier implementations (I
> > didn't check), but it does lessen flexibility?
> 
> In the normal page case, handle_pte_fault calls do_wp_page inside a spinlock on 
> ptl = pte_lockptr(mm, pmd), which uses mm->page_table_lock if USE_SPLIT_PTLOCKS 
> is not defined.
> 
> I don't understand what you mean by lessen flexibilty.

Well, specifically it means that
mmu_notifier_invalidate_range_start/end() implemetnations can no longer
take page_table_lock or any lock which nests outside page_table_lock. 
That lessens flexibility.

As the other mmu_notifier_invalidate_range_start/end() callsite in this
function carefully nested those calls outside page_table_lock, perhaps
that was thought to be a significant thing.

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-06-02 23:33     ` Andrew Morton
@ 2010-06-03 17:36       ` Doug Doan
  2010-06-03 18:11         ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Doan @ 2010-06-03 17:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	mel@csn.ul.ie, Andrea Arcangeli

On 06/02/2010 04:33 PM, Andrew Morton wrote:
> On Wed, 2 Jun 2010 16:13:42 -0700
> Doug Doan<dougd@cray.com>  wrote:
>
>> On 06/01/2010 11:16 PM, Andrew Morton wrote:
>>> On Thu, 27 May 2010 13:43:00 -0700 Doug Doan<dougd@cray.com>   wrote:
>>>
>>>>
>>>> When a copy-on-write occurs, we take one of two paths in handle_mm_fault:
>>>> through handle_pte_fault for normal pages, or through hugetlb_fault for huge pages.
>>>>
>>>> In the normal page case, we eventually get to do_wp_page and call mmu notifiers
>>>> via ptep_clear_flush_notify. There is no callout to the mmmu notifiers in the
>>>> huge page case. This patch fixes that.
>>>>
>>>> Signed-off-by: Doug Doan<dougd@cray.com>
>>>> ---
>>>>
>>>> [patch  text/plain (802B)]
>>>> --- mm/hugetlb.c.orig	2010-05-27 13:07:58.569546314 -0700
>>>> +++ mm/hugetlb.c	2010-05-26 14:41:06.449296524 -0700
>>>
>>> (In patch -p1 form, please.  So a/mm/hugetlb.c)
>>>
>>>> @@ -2345,11 +2345,17 @@ retry_avoidcopy:
>>>>    	ptep = huge_pte_offset(mm, address&   huge_page_mask(h));
>>>>    	if (likely(pte_same(huge_ptep_get(ptep), pte))) {
>>>>    		/* Break COW */
>>>> +		mmu_notifier_invalidate_range_start(mm,
>>>> +			address&   huge_page_mask(h),
>>>> +			(address&   huge_page_mask(h)) + huge_page_size(h));
>>>>    		huge_ptep_clear_flush(vma, address, ptep);
>>>>    		set_huge_pte_at(mm, address, ptep,
>>>>    				make_huge_pte(vma, new_page, 1));
>>>>    		/* Make the old page be freed below */
>>>>    		new_page = old_page;
>>>> +		mmu_notifier_invalidate_range_end(mm,
>>>> +			address&   huge_page_mask(h),
>>>> +			(address&   huge_page_mask(h)) + huge_page_size(h));
>>>>    	}
>>>>    	page_cache_release(new_page);
>>>>    	page_cache_release(old_page);
>>>
>>> This causes mmu_notifier_invalidate_range_start() to be called under
>>> page_table_lock.  The immediately preceding code seems to take some
>>> care to avoid doing that.  I took a quick look at other callsites and
>>> cannot immediately see other cases where
>>> mmu_notifier_invalidate_range_start/end() are called under that lock.
>>>
>>> This may not introduce bugs with current notifier implementations (I
>>> didn't check), but it does lessen flexibility?
>>
>> In the normal page case, handle_pte_fault calls do_wp_page inside a spinlock on
>> ptl = pte_lockptr(mm, pmd), which uses mm->page_table_lock if USE_SPLIT_PTLOCKS
>> is not defined.
>>
>> I don't understand what you mean by lessen flexibilty.
>
> Well, specifically it means that
> mmu_notifier_invalidate_range_start/end() implemetnations can no longer
> take page_table_lock or any lock which nests outside page_table_lock.
> That lessens flexibility.
>
> As the other mmu_notifier_invalidate_range_start/end() callsite in this
> function carefully nested those calls outside page_table_lock, perhaps
> that was thought to be a significant thing.

Here's my rationale: for the normal page case, the invalidation call is done 
inside a page_table_lock, so the same should also be done in the huge page case. 
Does it really make sense to call invalidation on one hugepage and have another 
call invalidate the same hugepage while the first call is still not finished?

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-06-03 17:36       ` Doug Doan
@ 2010-06-03 18:11         ` Andrew Morton
  2010-06-03 18:35           ` Doug Doan
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2010-06-03 18:11 UTC (permalink / raw)
  To: Doug Doan
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	mel@csn.ul.ie, Andrea Arcangeli

On Thu, 3 Jun 2010 10:36:00 -0700
Doug Doan <dougd@cray.com> wrote:

> > Well, specifically it means that
> > mmu_notifier_invalidate_range_start/end() implemetnations can no longer
> > take page_table_lock or any lock which nests outside page_table_lock.
> > That lessens flexibility.
> >
> > As the other mmu_notifier_invalidate_range_start/end() callsite in this
> > function carefully nested those calls outside page_table_lock, perhaps
> > that was thought to be a significant thing.
> 
> Here's my rationale: for the normal page case, the invalidation call is done 
> inside a page_table_lock,

It is?  Where does that happen?

> so the same should also be done in the huge page case. 
> Does it really make sense to call invalidation on one hugepage and have another 
> call invalidate the same hugepage while the first call is still not finished?

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hugetlb: call mmu notifiers on hugepage cow
  2010-06-03 18:11         ` Andrew Morton
@ 2010-06-03 18:35           ` Doug Doan
  0 siblings, 0 replies; 10+ messages in thread
From: Doug Doan @ 2010-06-03 18:35 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	andi@firstfloor.org, lee.schermerhorn@hp.com, rientjes@google.com,
	mel@csn.ul.ie, Andrea Arcangeli

On 06/03/2010 11:11 AM, Andrew Morton wrote:
> On Thu, 3 Jun 2010 10:36:00 -0700
> Doug Doan<dougd@cray.com>  wrote:
>
>>> Well, specifically it means that
>>> mmu_notifier_invalidate_range_start/end() implemetnations can no longer
>>> take page_table_lock or any lock which nests outside page_table_lock.
>>> That lessens flexibility.
>>>
>>> As the other mmu_notifier_invalidate_range_start/end() callsite in this
>>> function carefully nested those calls outside page_table_lock, perhaps
>>> that was thought to be a significant thing.
>>
>> Here's my rationale: for the normal page case, the invalidation call is done
>> inside a page_table_lock,
>
> It is?  Where does that happen?

handle_pte_fault() acquires the lock before calling do_wp_page():

         ptl = pte_lockptr(mm, pmd);
         spin_lock(ptl);
         if (unlikely(!pte_same(*pte, entry)))
                 goto unlock;
         if (flags & FAULT_FLAG_WRITE) {
                 if (!pte_write(entry))
                         return do_wp_page(mm, vma, address,
                                         pte, pmd, ptl, entry);
                 entry = pte_mkdirty(entry);
         }

do_wp_page() calls set_pte_at_notify(), which either calls 
mmu_notifier_change_pte() or mmu_notifier_invalidate_page().

>
>> so the same should also be done in the huge page case.
>> Does it really make sense to call invalidation on one hugepage and have another
>> call invalidate the same hugepage while the first call is still not finished?

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-06-03 18:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-27 20:43 [PATCH] hugetlb: call mmu notifiers on hugepage cow Doug Doan
2010-05-28  9:59 ` Mel Gorman
2010-05-28 16:39   ` Doug Doan
2010-05-28 17:34     ` Mel Gorman
2010-06-02  6:16 ` Andrew Morton
2010-06-02 23:13   ` Doug Doan
2010-06-02 23:33     ` Andrew Morton
2010-06-03 17:36       ` Doug Doan
2010-06-03 18:11         ` Andrew Morton
2010-06-03 18:35           ` Doug Doan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).