linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Reverts to address unixbench regression
@ 2016-06-14  8:42 Kirill A. Shutemov
  2016-06-14  8:42 ` [PATCH 1/2] Revert "mm: make faultaround produce old ptes" Kirill A. Shutemov
  2016-06-14  8:42 ` [PATCH 2/2] Revert "mm: disable fault around on emulated access bit architecture" Kirill A. Shutemov
  0 siblings, 2 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-06-14  8:42 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton
  Cc: Rik van Riel, Mel Gorman, Huang, Ying, Michal Hocko, Minchan Kim,
	Vinayak Menon, Dave Hansen, linux-kernel, linux-mm,
	Kirill A. Shutemov

Faultaround changes cause regression in unixbench, let's revert them.

Kirill A. Shutemov (2):
  Revert "mm: make faultaround produce old ptes"
  Revert "mm: disable fault around on emulated access bit architecture"

 include/linux/mm.h |  2 +-
 mm/filemap.c       |  2 +-
 mm/memory.c        | 31 +++++--------------------------
 3 files changed, 7 insertions(+), 28 deletions(-)

-- 
2.8.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] Revert "mm: make faultaround produce old ptes"
  2016-06-14  8:42 [PATCH 0/2] Reverts to address unixbench regression Kirill A. Shutemov
@ 2016-06-14  8:42 ` Kirill A. Shutemov
  2016-06-16 12:20   ` Michal Hocko
  2016-06-14  8:42 ` [PATCH 2/2] Revert "mm: disable fault around on emulated access bit architecture" Kirill A. Shutemov
  1 sibling, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-06-14  8:42 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton
  Cc: Rik van Riel, Mel Gorman, Huang, Ying, Michal Hocko, Minchan Kim,
	Vinayak Menon, Dave Hansen, linux-kernel, linux-mm,
	Kirill A. Shutemov

This reverts commit 5c0a85fad949212b3e059692deecdeed74ae7ec7.

The commit causes ~6% regression in unixbench.

Let's revert it for now and consider other solution for reclaim problem
later.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reported-by: "Huang, Ying" <ying.huang@intel.com>
---
 include/linux/mm.h |  2 +-
 mm/filemap.c       |  2 +-
 mm/memory.c        | 23 +++++------------------
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5df5feb49575..ece042dfe23c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -602,7 +602,7 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma)
 }
 
 void do_set_pte(struct vm_area_struct *vma, unsigned long address,
-		struct page *page, pte_t *pte, bool write, bool anon, bool old);
+		struct page *page, pte_t *pte, bool write, bool anon);
 #endif
 
 /*
diff --git a/mm/filemap.c b/mm/filemap.c
index 00ae878b2a38..20f3b1f33f0e 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2186,7 +2186,7 @@ repeat:
 		if (file->f_ra.mmap_miss > 0)
 			file->f_ra.mmap_miss--;
 		addr = address + (page->index - vmf->pgoff) * PAGE_SIZE;
-		do_set_pte(vma, addr, page, pte, false, false, true);
+		do_set_pte(vma, addr, page, pte, false, false);
 		unlock_page(page);
 		goto next;
 unlock:
diff --git a/mm/memory.c b/mm/memory.c
index 15322b73636b..61fe7e7b56bf 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2877,7 +2877,7 @@ static int __do_fault(struct vm_area_struct *vma, unsigned long address,
  * vm_ops->map_pages.
  */
 void do_set_pte(struct vm_area_struct *vma, unsigned long address,
-		struct page *page, pte_t *pte, bool write, bool anon, bool old)
+		struct page *page, pte_t *pte, bool write, bool anon)
 {
 	pte_t entry;
 
@@ -2885,8 +2885,6 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
 	entry = mk_pte(page, vma->vm_page_prot);
 	if (write)
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
-	if (old)
-		entry = pte_mkold(entry);
 	if (anon) {
 		inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
 		page_add_new_anon_rmap(page, vma, address, false);
@@ -3032,20 +3030,9 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 	 */
 	if (vma->vm_ops->map_pages && fault_around_bytes >> PAGE_SHIFT > 1) {
 		pte = pte_offset_map_lock(mm, pmd, address, &ptl);
-		if (!pte_same(*pte, orig_pte))
-			goto unlock_out;
 		do_fault_around(vma, address, pte, pgoff, flags);
-		/* Check if the fault is handled by faultaround */
-		if (!pte_same(*pte, orig_pte)) {
-			/*
-			 * Faultaround produce old pte, but the pte we've
-			 * handler fault for should be young.
-			 */
-			pte_t entry = pte_mkyoung(*pte);
-			if (ptep_set_access_flags(vma, address, pte, entry, 0))
-				update_mmu_cache(vma, address, pte);
+		if (!pte_same(*pte, orig_pte))
 			goto unlock_out;
-		}
 		pte_unmap_unlock(pte, ptl);
 	}
 
@@ -3060,7 +3047,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		put_page(fault_page);
 		return ret;
 	}
-	do_set_pte(vma, address, fault_page, pte, false, false, false);
+	do_set_pte(vma, address, fault_page, pte, false, false);
 	unlock_page(fault_page);
 unlock_out:
 	pte_unmap_unlock(pte, ptl);
@@ -3111,7 +3098,7 @@ static int do_cow_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		}
 		goto uncharge_out;
 	}
-	do_set_pte(vma, address, new_page, pte, true, true, false);
+	do_set_pte(vma, address, new_page, pte, true, true);
 	mem_cgroup_commit_charge(new_page, memcg, false, false);
 	lru_cache_add_active_or_unevictable(new_page, vma);
 	pte_unmap_unlock(pte, ptl);
@@ -3164,7 +3151,7 @@ static int do_shared_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		put_page(fault_page);
 		return ret;
 	}
-	do_set_pte(vma, address, fault_page, pte, true, false, false);
+	do_set_pte(vma, address, fault_page, pte, true, false);
 	pte_unmap_unlock(pte, ptl);
 
 	if (set_page_dirty(fault_page))
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] Revert "mm: disable fault around on emulated access bit architecture"
  2016-06-14  8:42 [PATCH 0/2] Reverts to address unixbench regression Kirill A. Shutemov
  2016-06-14  8:42 ` [PATCH 1/2] Revert "mm: make faultaround produce old ptes" Kirill A. Shutemov
@ 2016-06-14  8:42 ` Kirill A. Shutemov
  1 sibling, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-06-14  8:42 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton
  Cc: Rik van Riel, Mel Gorman, Huang, Ying, Michal Hocko, Minchan Kim,
	Vinayak Menon, Dave Hansen, linux-kernel, linux-mm,
	Kirill A. Shutemov

This reverts commit d0834a6c2c5b0c76cfb806bd7dba6556d8b4edbb.

After revert of 5c0a85fad949 ("mm: make faultaround produce old ptes")
faultaround doesn't have dependencies on hardware accessed bit, so let's
revert this one too.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 mm/memory.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 61fe7e7b56bf..cd1f29e4897e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2898,16 +2898,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
 	update_mmu_cache(vma, address, pte);
 }
 
-/*
- * If architecture emulates "accessed" or "young" bit without HW support,
- * there is no much gain with fault_around.
- */
 static unsigned long fault_around_bytes __read_mostly =
-#ifndef __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
-	PAGE_SIZE;
-#else
 	rounddown_pow_of_two(65536);
-#endif
 
 #ifdef CONFIG_DEBUG_FS
 static int fault_around_bytes_get(void *data, u64 *val)
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Revert "mm: make faultaround produce old ptes"
  2016-06-14  8:42 ` [PATCH 1/2] Revert "mm: make faultaround produce old ptes" Kirill A. Shutemov
@ 2016-06-16 12:20   ` Michal Hocko
  2016-06-16 12:27     ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2016-06-16 12:20 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Linus Torvalds, Andrew Morton, Rik van Riel, Mel Gorman, Huang,
	Ying, Minchan Kim, Vinayak Menon, Dave Hansen, linux-kernel,
	linux-mm

On Tue 14-06-16 11:42:29, Kirill A. Shutemov wrote:
> This reverts commit 5c0a85fad949212b3e059692deecdeed74ae7ec7.
> 
> The commit causes ~6% regression in unixbench.

Is the regression fully explained? My understanding from the email
thread is that this is suspiciously too high. It is not like I would
be against the revert but having an explanation would be really
appreciated.

> Let's revert it for now and consider other solution for reclaim problem
> later.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reported-by: "Huang, Ying" <ying.huang@intel.com>
> ---
>  include/linux/mm.h |  2 +-
>  mm/filemap.c       |  2 +-
>  mm/memory.c        | 23 +++++------------------
>  3 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5df5feb49575..ece042dfe23c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -602,7 +602,7 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma)
>  }
>  
>  void do_set_pte(struct vm_area_struct *vma, unsigned long address,
> -		struct page *page, pte_t *pte, bool write, bool anon, bool old);
> +		struct page *page, pte_t *pte, bool write, bool anon);
>  #endif
>  
>  /*
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 00ae878b2a38..20f3b1f33f0e 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2186,7 +2186,7 @@ repeat:
>  		if (file->f_ra.mmap_miss > 0)
>  			file->f_ra.mmap_miss--;
>  		addr = address + (page->index - vmf->pgoff) * PAGE_SIZE;
> -		do_set_pte(vma, addr, page, pte, false, false, true);
> +		do_set_pte(vma, addr, page, pte, false, false);
>  		unlock_page(page);
>  		goto next;
>  unlock:
> diff --git a/mm/memory.c b/mm/memory.c
> index 15322b73636b..61fe7e7b56bf 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2877,7 +2877,7 @@ static int __do_fault(struct vm_area_struct *vma, unsigned long address,
>   * vm_ops->map_pages.
>   */
>  void do_set_pte(struct vm_area_struct *vma, unsigned long address,
> -		struct page *page, pte_t *pte, bool write, bool anon, bool old)
> +		struct page *page, pte_t *pte, bool write, bool anon)
>  {
>  	pte_t entry;
>  
> @@ -2885,8 +2885,6 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
>  	entry = mk_pte(page, vma->vm_page_prot);
>  	if (write)
>  		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> -	if (old)
> -		entry = pte_mkold(entry);
>  	if (anon) {
>  		inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
>  		page_add_new_anon_rmap(page, vma, address, false);
> @@ -3032,20 +3030,9 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>  	 */
>  	if (vma->vm_ops->map_pages && fault_around_bytes >> PAGE_SHIFT > 1) {
>  		pte = pte_offset_map_lock(mm, pmd, address, &ptl);
> -		if (!pte_same(*pte, orig_pte))
> -			goto unlock_out;
>  		do_fault_around(vma, address, pte, pgoff, flags);
> -		/* Check if the fault is handled by faultaround */
> -		if (!pte_same(*pte, orig_pte)) {
> -			/*
> -			 * Faultaround produce old pte, but the pte we've
> -			 * handler fault for should be young.
> -			 */
> -			pte_t entry = pte_mkyoung(*pte);
> -			if (ptep_set_access_flags(vma, address, pte, entry, 0))
> -				update_mmu_cache(vma, address, pte);
> +		if (!pte_same(*pte, orig_pte))
>  			goto unlock_out;
> -		}
>  		pte_unmap_unlock(pte, ptl);
>  	}
>  
> @@ -3060,7 +3047,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>  		put_page(fault_page);
>  		return ret;
>  	}
> -	do_set_pte(vma, address, fault_page, pte, false, false, false);
> +	do_set_pte(vma, address, fault_page, pte, false, false);
>  	unlock_page(fault_page);
>  unlock_out:
>  	pte_unmap_unlock(pte, ptl);
> @@ -3111,7 +3098,7 @@ static int do_cow_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>  		}
>  		goto uncharge_out;
>  	}
> -	do_set_pte(vma, address, new_page, pte, true, true, false);
> +	do_set_pte(vma, address, new_page, pte, true, true);
>  	mem_cgroup_commit_charge(new_page, memcg, false, false);
>  	lru_cache_add_active_or_unevictable(new_page, vma);
>  	pte_unmap_unlock(pte, ptl);
> @@ -3164,7 +3151,7 @@ static int do_shared_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>  		put_page(fault_page);
>  		return ret;
>  	}
> -	do_set_pte(vma, address, fault_page, pte, true, false, false);
> +	do_set_pte(vma, address, fault_page, pte, true, false);
>  	pte_unmap_unlock(pte, ptl);
>  
>  	if (set_page_dirty(fault_page))
> -- 
> 2.8.1
> 

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Revert "mm: make faultaround produce old ptes"
  2016-06-16 12:20   ` Michal Hocko
@ 2016-06-16 12:27     ` Kirill A. Shutemov
  2016-06-16 15:10       ` Michal Hocko
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2016-06-16 12:27 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Linus Torvalds, Andrew Morton, Rik van Riel, Mel Gorman, Huang,
	Ying, Minchan Kim, Vinayak Menon, Dave Hansen, linux-kernel,
	linux-mm

On Thu, Jun 16, 2016 at 02:20:02PM +0200, Michal Hocko wrote:
> On Tue 14-06-16 11:42:29, Kirill A. Shutemov wrote:
> > This reverts commit 5c0a85fad949212b3e059692deecdeed74ae7ec7.
> > 
> > The commit causes ~6% regression in unixbench.
> 
> Is the regression fully explained? My understanding from the email
> thread is that this is suspiciously too high. It is not like I would
> be against the revert but having an explanation would be really
> appreciated.

My understanding is that it's overhead on setting accessed bit:

http://lkml.kernel.org/r/20160613125248.GA30109@black.fi.intel.com

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Revert "mm: make faultaround produce old ptes"
  2016-06-16 12:27     ` Kirill A. Shutemov
@ 2016-06-16 15:10       ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2016-06-16 15:10 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Linus Torvalds, Andrew Morton, Rik van Riel, Mel Gorman, Huang,
	Ying, Minchan Kim, Vinayak Menon, Dave Hansen, linux-kernel,
	linux-mm

On Thu 16-06-16 15:27:35, Kirill A. Shutemov wrote:
> On Thu, Jun 16, 2016 at 02:20:02PM +0200, Michal Hocko wrote:
> > On Tue 14-06-16 11:42:29, Kirill A. Shutemov wrote:
> > > This reverts commit 5c0a85fad949212b3e059692deecdeed74ae7ec7.
> > > 
> > > The commit causes ~6% regression in unixbench.
> > 
> > Is the regression fully explained? My understanding from the email
> > thread is that this is suspiciously too high. It is not like I would
> > be against the revert but having an explanation would be really
> > appreciated.
> 
> My understanding is that it's overhead on setting accessed bit:
> 
> http://lkml.kernel.org/r/20160613125248.GA30109@black.fi.intel.com

But those numbers cannot explain the regression completely AFAIU. It
smells like something else is going on. Anyway, as I've said I do not
have anything against the revert just more than "unixbench regresses"
would be nice.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-16 15:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14  8:42 [PATCH 0/2] Reverts to address unixbench regression Kirill A. Shutemov
2016-06-14  8:42 ` [PATCH 1/2] Revert "mm: make faultaround produce old ptes" Kirill A. Shutemov
2016-06-16 12:20   ` Michal Hocko
2016-06-16 12:27     ` Kirill A. Shutemov
2016-06-16 15:10       ` Michal Hocko
2016-06-14  8:42 ` [PATCH 2/2] Revert "mm: disable fault around on emulated access bit architecture" Kirill A. Shutemov

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