linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* mm/memory.c: Add update local tlb for smp race
@ 2020-05-14  6:50 Bibo Mao
  2020-05-15 13:50 ` David Hildenbrand
  0 siblings, 1 reply; 4+ messages in thread
From: Bibo Mao @ 2020-05-14  6:50 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-kernel, linux-mips

If there are two threads hitting page fault at the address, one
thread updates pte entry and local tlb, the other thread can update
local tlb also, rather than give up and let page fault happening
again.

	modified:   mm/memory.c

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 mm/memory.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index f703fe8..3a741ce 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2436,11 +2436,10 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
 		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
 			/*
 			 * Other thread has already handled the fault
-			 * and we don't need to do anything. If it's
-			 * not the case, the fault will be triggered
-			 * again on the same address.
+			 * and update local tlb only
 			 */
 			ret = false;
+			update_mmu_cache(vma, addr, vmf->pte);
 			goto pte_unlock;
 		}
 
@@ -2463,8 +2462,9 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
 		vmf->pte = pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl);
 		locked = true;
 		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
-			/* The PTE changed under us. Retry page fault. */
+			/* The PTE changed under us. update local tlb */
 			ret = false;
+			update_mmu_cache(vma, addr, vmf->pte);
 			goto pte_unlock;
 		}
 
@@ -2704,6 +2704,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
 		}
 		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
 		entry = mk_pte(new_page, vma->vm_page_prot);
+		entry = pte_mkyoung(entry);
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
 		/*
 		 * Clear the pte entry and flush it first, before updating the
@@ -2752,6 +2753,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
 		new_page = old_page;
 		page_copied = 1;
 	} else {
+		update_mmu_cache(vma, vmf->address, vmf->pte);
 		mem_cgroup_cancel_charge(new_page, memcg, false);
 	}
 
@@ -2812,6 +2814,7 @@ vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf)
 	 * pte_offset_map_lock.
 	 */
 	if (!pte_same(*vmf->pte, vmf->orig_pte)) {
+		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
 		return VM_FAULT_NOPAGE;
 	}
@@ -2936,6 +2939,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
 			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
 					vmf->address, &vmf->ptl);
 			if (!pte_same(*vmf->pte, vmf->orig_pte)) {
+				update_mmu_cache(vma, vmf->address, vmf->pte);
 				unlock_page(vmf->page);
 				pte_unmap_unlock(vmf->pte, vmf->ptl);
 				put_page(vmf->page);
@@ -3341,8 +3345,10 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
 						vma->vm_page_prot));
 		vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
 				vmf->address, &vmf->ptl);
-		if (!pte_none(*vmf->pte))
+		if (!pte_none(*vmf->pte)) {
+			update_mmu_cache(vma, vmf->address, vmf->pte);
 			goto unlock;
+		}
 		ret = check_stable_address_space(vma->vm_mm);
 		if (ret)
 			goto unlock;
@@ -3373,13 +3379,16 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
 	__SetPageUptodate(page);
 
 	entry = mk_pte(page, vma->vm_page_prot);
+	entry = pte_mkyoung(entry);
 	if (vma->vm_flags & VM_WRITE)
 		entry = pte_mkwrite(pte_mkdirty(entry));
 
 	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
 			&vmf->ptl);
-	if (!pte_none(*vmf->pte))
+	if (!pte_none(*vmf->pte)) {
+		update_mmu_cache(vma, vmf->address, vmf->pte);
 		goto release;
+	}
 
 	ret = check_stable_address_space(vma->vm_mm);
 	if (ret)
@@ -3646,11 +3655,14 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
 	}
 
 	/* Re-check under ptl */
-	if (unlikely(!pte_none(*vmf->pte)))
+	if (unlikely(!pte_none(*vmf->pte))) {
+		update_mmu_cache(vma, vmf->address, vmf->pte);
 		return VM_FAULT_NOPAGE;
+	}
 
 	flush_icache_page(vma, page);
 	entry = mk_pte(page, vma->vm_page_prot);
+	entry = pte_mkyoung(entry);
 	if (write)
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
 	/* copy-on-write page */
@@ -4224,8 +4236,10 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
 	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
 	spin_lock(vmf->ptl);
 	entry = vmf->orig_pte;
-	if (unlikely(!pte_same(*vmf->pte, entry)))
+	if (unlikely(!pte_same(*vmf->pte, entry))) {
+		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
 		goto unlock;
+	}
 	if (vmf->flags & FAULT_FLAG_WRITE) {
 		if (!pte_write(entry))
 			return do_wp_page(vmf);
-- 
1.8.3.1


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

* Re: mm/memory.c: Add update local tlb for smp race
  2020-05-14  6:50 mm/memory.c: Add update local tlb for smp race Bibo Mao
@ 2020-05-15 13:50 ` David Hildenbrand
  2020-05-16  9:34   ` maobibo
  0 siblings, 1 reply; 4+ messages in thread
From: David Hildenbrand @ 2020-05-15 13:50 UTC (permalink / raw)
  To: Bibo Mao, Andrew Morton; +Cc: linux-mm, linux-kernel, linux-mips

On 14.05.20 08:50, Bibo Mao wrote:
> If there are two threads hitting page fault at the address, one
> thread updates pte entry and local tlb, the other thread can update
> local tlb also, rather than give up and let page fault happening
> again.

Let me suggest

"mm/memory: optimize concurrent page faults at same address

If two threads concurrently fault at the same address, the thread that
won the race updates the PTE and its local TLB. For now, the other
thread gives up, simply does nothing, and continues.

It could happen that this second thread triggers another fault, whereby
it only updates its local TLB while handling the fault. Instead of
triggering another fault, let's directly update the local TLB of the
second thread.
"

If I got the intention of this patch correctly.

Are there any performance numbers to support this patch?

(I can't say too much about the correctness and/or usefulness of this patch)

> 
> 	modified:   mm/memory.c

This does not belong into a patch description.


> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  mm/memory.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index f703fe8..3a741ce 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2436,11 +2436,10 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
>  			/*
>  			 * Other thread has already handled the fault
> -			 * and we don't need to do anything. If it's
> -			 * not the case, the fault will be triggered
> -			 * again on the same address.
> +			 * and update local tlb only
>  			 */
>  			ret = false;
> +			update_mmu_cache(vma, addr, vmf->pte);
>  			goto pte_unlock;
>  		}
>  
> @@ -2463,8 +2462,9 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>  		vmf->pte = pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl);
>  		locked = true;
>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
> -			/* The PTE changed under us. Retry page fault. */
> +			/* The PTE changed under us. update local tlb */
>  			ret = false;
> +			update_mmu_cache(vma, addr, vmf->pte);
>  			goto pte_unlock;
>  		}
>  
> @@ -2704,6 +2704,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>  		}
>  		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
>  		entry = mk_pte(new_page, vma->vm_page_prot);
> +		entry = pte_mkyoung(entry);
>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  		/*
>  		 * Clear the pte entry and flush it first, before updating the
> @@ -2752,6 +2753,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>  		new_page = old_page;
>  		page_copied = 1;
>  	} else {
> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>  		mem_cgroup_cancel_charge(new_page, memcg, false);
>  	}
>  
> @@ -2812,6 +2814,7 @@ vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf)
>  	 * pte_offset_map_lock.
>  	 */
>  	if (!pte_same(*vmf->pte, vmf->orig_pte)) {
> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>  		pte_unmap_unlock(vmf->pte, vmf->ptl);
>  		return VM_FAULT_NOPAGE;
>  	}
> @@ -2936,6 +2939,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
>  			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>  					vmf->address, &vmf->ptl);
>  			if (!pte_same(*vmf->pte, vmf->orig_pte)) {
> +				update_mmu_cache(vma, vmf->address, vmf->pte);
>  				unlock_page(vmf->page);
>  				pte_unmap_unlock(vmf->pte, vmf->ptl);
>  				put_page(vmf->page);
> @@ -3341,8 +3345,10 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>  						vma->vm_page_prot));
>  		vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>  				vmf->address, &vmf->ptl);
> -		if (!pte_none(*vmf->pte))
> +		if (!pte_none(*vmf->pte)) {
> +			update_mmu_cache(vma, vmf->address, vmf->pte);
>  			goto unlock;
> +		}
>  		ret = check_stable_address_space(vma->vm_mm);
>  		if (ret)
>  			goto unlock;
> @@ -3373,13 +3379,16 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>  	__SetPageUptodate(page);
>  
>  	entry = mk_pte(page, vma->vm_page_prot);
> +	entry = pte_mkyoung(entry);
>  	if (vma->vm_flags & VM_WRITE)
>  		entry = pte_mkwrite(pte_mkdirty(entry));
>  
>  	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
>  			&vmf->ptl);
> -	if (!pte_none(*vmf->pte))
> +	if (!pte_none(*vmf->pte)) {
> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>  		goto release;
> +	}
>  
>  	ret = check_stable_address_space(vma->vm_mm);
>  	if (ret)
> @@ -3646,11 +3655,14 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
>  	}
>  
>  	/* Re-check under ptl */
> -	if (unlikely(!pte_none(*vmf->pte)))
> +	if (unlikely(!pte_none(*vmf->pte))) {
> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>  		return VM_FAULT_NOPAGE;
> +	}
>  
>  	flush_icache_page(vma, page);
>  	entry = mk_pte(page, vma->vm_page_prot);
> +	entry = pte_mkyoung(entry);
>  	if (write)
>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  	/* copy-on-write page */
> @@ -4224,8 +4236,10 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>  	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
>  	spin_lock(vmf->ptl);
>  	entry = vmf->orig_pte;
> -	if (unlikely(!pte_same(*vmf->pte, entry)))
> +	if (unlikely(!pte_same(*vmf->pte, entry))) {
> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>  		goto unlock;
> +	}
>  	if (vmf->flags & FAULT_FLAG_WRITE) {
>  		if (!pte_write(entry))
>  			return do_wp_page(vmf);
> 


-- 
Thanks,

David / dhildenb


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

* Re: mm/memory.c: Add update local tlb for smp race
  2020-05-15 13:50 ` David Hildenbrand
@ 2020-05-16  9:34   ` maobibo
  2020-05-18  3:33     ` maobibo
  0 siblings, 1 reply; 4+ messages in thread
From: maobibo @ 2020-05-16  9:34 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton; +Cc: linux-mm, linux-kernel, linux-mips



On 05/15/2020 09:50 PM, David Hildenbrand wrote:
> On 14.05.20 08:50, Bibo Mao wrote:
>> If there are two threads hitting page fault at the address, one
>> thread updates pte entry and local tlb, the other thread can update
>> local tlb also, rather than give up and let page fault happening
>> again.
> 
> Let me suggest
> 
> "mm/memory: optimize concurrent page faults at same address
> 
> If two threads concurrently fault at the same address, the thread that
> won the race updates the PTE and its local TLB. For now, the other
> thread gives up, simply does nothing, and continues.
> 
> It could happen that this second thread triggers another fault, whereby
> it only updates its local TLB while handling the fault. Instead of
> triggering another fault, let's directly update the local TLB of the
> second thread.
> "
> 
> If I got the intention of this patch correctly.
> 
> Are there any performance numbers to support this patch?
> 
> (I can't say too much about the correctness and/or usefulness of this patch)

yes, that is the situation. On MIPS platform software can update TLB,
so update_mmu_cache is used here. This does not happen frequently, and with the three series patches in later mail. I test lat_pagefault in lmbench, here is is result:

with these three series patches, 
# ./lat_pagefault  -N 10  /tmp/1 
Pagefaults on /tmp/1: 1.4973 microseconds
# ./lat_pagefault -P 4 -N 10  /tmp/1 
Pagefaults on /tmp/1: 1.5716 microseconds

original version, without these three series patch
#  ./lat_pagefault  -N 10  /tmp/1 
Pagefaults on /tmp/1: 1.6489 microseconds
# ./lat_pagefault -P 4 -N 10  /tmp/1
Pagefaults on /tmp/1: 1.7214 microseconds

>>
>> 	modified:   mm/memory.c
> 
> This does not belong into a patch description.

well, I will modify the patch description.

regards
bibo,mao


> 
> 
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  mm/memory.c | 30 ++++++++++++++++++++++--------
>>  1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index f703fe8..3a741ce 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2436,11 +2436,10 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
>>  			/*
>>  			 * Other thread has already handled the fault
>> -			 * and we don't need to do anything. If it's
>> -			 * not the case, the fault will be triggered
>> -			 * again on the same address.
>> +			 * and update local tlb only
>>  			 */
>>  			ret = false;
>> +			update_mmu_cache(vma, addr, vmf->pte);
>>  			goto pte_unlock;
>>  		}
>>  
>> @@ -2463,8 +2462,9 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>>  		vmf->pte = pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl);
>>  		locked = true;
>>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
>> -			/* The PTE changed under us. Retry page fault. */
>> +			/* The PTE changed under us. update local tlb */
>>  			ret = false;
>> +			update_mmu_cache(vma, addr, vmf->pte);
>>  			goto pte_unlock;
>>  		}
>>  
>> @@ -2704,6 +2704,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>>  		}
>>  		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
>>  		entry = mk_pte(new_page, vma->vm_page_prot);
>> +		entry = pte_mkyoung(entry);
>>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>  		/*
>>  		 * Clear the pte entry and flush it first, before updating the
>> @@ -2752,6 +2753,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>>  		new_page = old_page;
>>  		page_copied = 1;
>>  	} else {
>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>  		mem_cgroup_cancel_charge(new_page, memcg, false);
>>  	}
>>  
>> @@ -2812,6 +2814,7 @@ vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf)
>>  	 * pte_offset_map_lock.
>>  	 */
>>  	if (!pte_same(*vmf->pte, vmf->orig_pte)) {
>> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>>  		pte_unmap_unlock(vmf->pte, vmf->ptl);
>>  		return VM_FAULT_NOPAGE;
>>  	}
>> @@ -2936,6 +2939,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
>>  			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>  					vmf->address, &vmf->ptl);
>>  			if (!pte_same(*vmf->pte, vmf->orig_pte)) {
>> +				update_mmu_cache(vma, vmf->address, vmf->pte);
>>  				unlock_page(vmf->page);
>>  				pte_unmap_unlock(vmf->pte, vmf->ptl);
>>  				put_page(vmf->page);
>> @@ -3341,8 +3345,10 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>>  						vma->vm_page_prot));
>>  		vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>  				vmf->address, &vmf->ptl);
>> -		if (!pte_none(*vmf->pte))
>> +		if (!pte_none(*vmf->pte)) {
>> +			update_mmu_cache(vma, vmf->address, vmf->pte);
>>  			goto unlock;
>> +		}
>>  		ret = check_stable_address_space(vma->vm_mm);
>>  		if (ret)
>>  			goto unlock;
>> @@ -3373,13 +3379,16 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>>  	__SetPageUptodate(page);
>>  
>>  	entry = mk_pte(page, vma->vm_page_prot);
>> +	entry = pte_mkyoung(entry);
>>  	if (vma->vm_flags & VM_WRITE)
>>  		entry = pte_mkwrite(pte_mkdirty(entry));
>>  
>>  	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
>>  			&vmf->ptl);
>> -	if (!pte_none(*vmf->pte))
>> +	if (!pte_none(*vmf->pte)) {
>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>  		goto release;
>> +	}
>>  
>>  	ret = check_stable_address_space(vma->vm_mm);
>>  	if (ret)
>> @@ -3646,11 +3655,14 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
>>  	}
>>  
>>  	/* Re-check under ptl */
>> -	if (unlikely(!pte_none(*vmf->pte)))
>> +	if (unlikely(!pte_none(*vmf->pte))) {
>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>  		return VM_FAULT_NOPAGE;
>> +	}
>>  
>>  	flush_icache_page(vma, page);
>>  	entry = mk_pte(page, vma->vm_page_prot);
>> +	entry = pte_mkyoung(entry);
>>  	if (write)
>>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>  	/* copy-on-write page */
>> @@ -4224,8 +4236,10 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>  	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
>>  	spin_lock(vmf->ptl);
>>  	entry = vmf->orig_pte;
>> -	if (unlikely(!pte_same(*vmf->pte, entry)))
>> +	if (unlikely(!pte_same(*vmf->pte, entry))) {
>> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>>  		goto unlock;
>> +	}
>>  	if (vmf->flags & FAULT_FLAG_WRITE) {
>>  		if (!pte_write(entry))
>>  			return do_wp_page(vmf);
>>
> 
> 


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

* Re: mm/memory.c: Add update local tlb for smp race
  2020-05-16  9:34   ` maobibo
@ 2020-05-18  3:33     ` maobibo
  0 siblings, 0 replies; 4+ messages in thread
From: maobibo @ 2020-05-18  3:33 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton; +Cc: linux-mm, linux-kernel, linux-mips



On 05/16/2020 05:34 PM, maobibo wrote:
> 
> 
> On 05/15/2020 09:50 PM, David Hildenbrand wrote:
>> On 14.05.20 08:50, Bibo Mao wrote:
>>> If there are two threads hitting page fault at the address, one
>>> thread updates pte entry and local tlb, the other thread can update
>>> local tlb also, rather than give up and let page fault happening
>>> again.
>>
>> Let me suggest
>>
>> "mm/memory: optimize concurrent page faults at same address
>>
>> If two threads concurrently fault at the same address, the thread that
>> won the race updates the PTE and its local TLB. For now, the other
>> thread gives up, simply does nothing, and continues.
>>
>> It could happen that this second thread triggers another fault, whereby
>> it only updates its local TLB while handling the fault. Instead of
>> triggering another fault, let's directly update the local TLB of the
>> second thread.
>> "
>>
>> If I got the intention of this patch correctly.
>>
>> Are there any performance numbers to support this patch?
>>
>> (I can't say too much about the correctness and/or usefulness of this patch)
> 
> yes, that is the situation. On MIPS platform software can update TLB,
> so update_mmu_cache is used here. This does not happen frequently, and with the three series patches in later mail. I test lat_pagefault in lmbench, here is is result:
> 
> with these three series patches, 
> # ./lat_pagefault  -N 10  /tmp/1 
> Pagefaults on /tmp/1: 1.4973 microseconds
> # ./lat_pagefault -P 4 -N 10  /tmp/1 
> Pagefaults on /tmp/1: 1.5716 microseconds
> 
> original version, without these three series patch
> #  ./lat_pagefault  -N 10  /tmp/1 
> Pagefaults on /tmp/1: 1.6489 microseconds
> # ./lat_pagefault -P 4 -N 10  /tmp/1
> Pagefaults on /tmp/1: 1.7214 microseconds
> 

I tested the three patches one by one, there is no obvious improvement with
lat_pagefault case, I guess that it happens seldom where multiple threads access
the same page at the same time. 

The improvement is because of another modification where pte_mkyoung is added
to get readable privilege on MIPS system.

regards
bibo, mao

>>>
>>> 	modified:   mm/memory.c
>>
>> This does not belong into a patch description.
> 
> well, I will modify the patch description.
> 
> regards
> bibo,mao
> 
> 
>>
>>
>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>> ---
>>>  mm/memory.c | 30 ++++++++++++++++++++++--------
>>>  1 file changed, 22 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index f703fe8..3a741ce 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -2436,11 +2436,10 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>>>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
>>>  			/*
>>>  			 * Other thread has already handled the fault
>>> -			 * and we don't need to do anything. If it's
>>> -			 * not the case, the fault will be triggered
>>> -			 * again on the same address.
>>> +			 * and update local tlb only
>>>  			 */
>>>  			ret = false;
>>> +			update_mmu_cache(vma, addr, vmf->pte);
>>>  			goto pte_unlock;
>>>  		}
>>>  
>>> @@ -2463,8 +2462,9 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>>>  		vmf->pte = pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl);
>>>  		locked = true;
>>>  		if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) {
>>> -			/* The PTE changed under us. Retry page fault. */
>>> +			/* The PTE changed under us. update local tlb */
>>>  			ret = false;
>>> +			update_mmu_cache(vma, addr, vmf->pte);
>>>  			goto pte_unlock;
>>>  		}
>>>  
>>> @@ -2704,6 +2704,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>>>  		}
>>>  		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
>>>  		entry = mk_pte(new_page, vma->vm_page_prot);
>>> +		entry = pte_mkyoung(entry);
>>>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>>  		/*
>>>  		 * Clear the pte entry and flush it first, before updating the
>>> @@ -2752,6 +2753,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>>>  		new_page = old_page;
>>>  		page_copied = 1;
>>>  	} else {
>>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>>  		mem_cgroup_cancel_charge(new_page, memcg, false);
>>>  	}
>>>  
>>> @@ -2812,6 +2814,7 @@ vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf)
>>>  	 * pte_offset_map_lock.
>>>  	 */
>>>  	if (!pte_same(*vmf->pte, vmf->orig_pte)) {
>>> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>>>  		pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>  		return VM_FAULT_NOPAGE;
>>>  	}
>>> @@ -2936,6 +2939,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
>>>  			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>>  					vmf->address, &vmf->ptl);
>>>  			if (!pte_same(*vmf->pte, vmf->orig_pte)) {
>>> +				update_mmu_cache(vma, vmf->address, vmf->pte);
>>>  				unlock_page(vmf->page);
>>>  				pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>  				put_page(vmf->page);
>>> @@ -3341,8 +3345,10 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>>>  						vma->vm_page_prot));
>>>  		vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
>>>  				vmf->address, &vmf->ptl);
>>> -		if (!pte_none(*vmf->pte))
>>> +		if (!pte_none(*vmf->pte)) {
>>> +			update_mmu_cache(vma, vmf->address, vmf->pte);
>>>  			goto unlock;
>>> +		}
>>>  		ret = check_stable_address_space(vma->vm_mm);
>>>  		if (ret)
>>>  			goto unlock;
>>> @@ -3373,13 +3379,16 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>>>  	__SetPageUptodate(page);
>>>  
>>>  	entry = mk_pte(page, vma->vm_page_prot);
>>> +	entry = pte_mkyoung(entry);
>>>  	if (vma->vm_flags & VM_WRITE)
>>>  		entry = pte_mkwrite(pte_mkdirty(entry));
>>>  
>>>  	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
>>>  			&vmf->ptl);
>>> -	if (!pte_none(*vmf->pte))
>>> +	if (!pte_none(*vmf->pte)) {
>>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>>  		goto release;
>>> +	}
>>>  
>>>  	ret = check_stable_address_space(vma->vm_mm);
>>>  	if (ret)
>>> @@ -3646,11 +3655,14 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
>>>  	}
>>>  
>>>  	/* Re-check under ptl */
>>> -	if (unlikely(!pte_none(*vmf->pte)))
>>> +	if (unlikely(!pte_none(*vmf->pte))) {
>>> +		update_mmu_cache(vma, vmf->address, vmf->pte);
>>>  		return VM_FAULT_NOPAGE;
>>> +	}
>>>  
>>>  	flush_icache_page(vma, page);
>>>  	entry = mk_pte(page, vma->vm_page_prot);
>>> +	entry = pte_mkyoung(entry);
>>>  	if (write)
>>>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>>  	/* copy-on-write page */
>>> @@ -4224,8 +4236,10 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>>  	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
>>>  	spin_lock(vmf->ptl);
>>>  	entry = vmf->orig_pte;
>>> -	if (unlikely(!pte_same(*vmf->pte, entry)))
>>> +	if (unlikely(!pte_same(*vmf->pte, entry))) {
>>> +		update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
>>>  		goto unlock;
>>> +	}
>>>  	if (vmf->flags & FAULT_FLAG_WRITE) {
>>>  		if (!pte_write(entry))
>>>  			return do_wp_page(vmf);
>>>
>>
>>


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

end of thread, other threads:[~2020-05-18  3:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14  6:50 mm/memory.c: Add update local tlb for smp race Bibo Mao
2020-05-15 13:50 ` David Hildenbrand
2020-05-16  9:34   ` maobibo
2020-05-18  3:33     ` maobibo

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