[PATCH 1/2] mm/memory: remove redundant pte_devmap case for copy_one_pte()

[PATCH 1/2] mm/memory: remove redundant pte_devmap case for copy_one_pte()

[qiwuchen55]

From: chenqiwu <chenqiwu@xiaomi.com>

As discussed in patch[1], special mapping ptes such as device
mapping ptes don't need to operate on their corresponding pages,
so the redundant pte_devmap case can be removed.

[1] https://patchwork.kernel.org/patch/11494691/

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
---
 mm/memory.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index f703fe8..2143827 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -802,8 +802,6 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
 		get_page(page);
 		page_dup_rmap(page, false);
 		rss[mm_counter(page)]++;
-	} else if (pte_devmap(pte)) {
-		page = pte_page(pte);
 	}
 
 out_set_pte:
-- 
1.9.1

[PATCH 2/2] mm/memory: put refcount of normal page in zap_pte_range()

[qiwuchen55]

From: chenqiwu <chenqiwu@xiaomi.com>

As discussed in patch[1], there is an imbalnace of normal
page refcount between copy_one_pte() and zap_pte_range().
This patch put the refcount of normal page back in
zap_pte_range() to fix the imbalance.

[1] https://patchwork.kernel.org/patch/11494691/

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
---
 mm/memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/memory.c b/mm/memory.c
index 2143827..ec8de9a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1088,6 +1088,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 			}
 			rss[mm_counter(page)]--;
 			page_remove_rmap(page, false);
+			put_page(page);
 			if (unlikely(page_mapcount(page) < 0))
 				print_bad_pte(vma, addr, ptent, page);
 			if (unlikely(__tlb_remove_page(tlb, page))) {
-- 
1.9.1

Re: [PATCH 1/2] mm/memory: remove redundant pte_devmap case for copy_one_pte()

[Matthew Wilcox]

On Sun, Apr 19, 2020 at 11:30:16AM +0800, qiwuchen55@gmail.com wrote:
> From: chenqiwu <chenqiwu@xiaomi.com>
> 
> As discussed in patch[1], special mapping ptes such as device
> mapping ptes don't need to operate on their corresponding pages,
> so the redundant pte_devmap case can be removed.
> 
> [1] https://patchwork.kernel.org/patch/11494691/

This is a terrible changelog.  What it should say is something like:

Since commit 25b2995a35b6 ("mm: remove MEMORY_DEVICE_PUBLIC support"),
the assignment to 'page' has been unnecessary.  Remove it.

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

> Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
> ---
>  mm/memory.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index f703fe8..2143827 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -802,8 +802,6 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
>  		get_page(page);
>  		page_dup_rmap(page, false);
>  		rss[mm_counter(page)]++;
> -	} else if (pte_devmap(pte)) {
> -		page = pte_page(pte);
>  	}
>  
>  out_set_pte:
> -- 
> 1.9.1
> 
> 

Re: [PATCH 2/2] mm/memory: put refcount of normal page in zap_pte_range()

[Matthew Wilcox]

On Sun, Apr 19, 2020 at 11:30:17AM +0800, qiwuchen55@gmail.com wrote:
> From: chenqiwu <chenqiwu@xiaomi.com>
> 
> As discussed in patch[1], there is an imbalnace of normal
> page refcount between copy_one_pte() and zap_pte_range().
> This patch put the refcount of normal page back in
> zap_pte_range() to fix the imbalance.

I think you misunderstood the discussion.  NAK.  Did you test this?

Re: [PATCH 1/2] mm/memory: remove redundant pte_devmap case for copy_one_pte()

[Michal Hocko]

On Sun 19-04-20 03:26:13, Matthew Wilcox wrote:
> On Sun, Apr 19, 2020 at 11:30:16AM +0800, qiwuchen55@gmail.com wrote:
> > From: chenqiwu <chenqiwu@xiaomi.com>
> > 
> > As discussed in patch[1], special mapping ptes such as device
> > mapping ptes don't need to operate on their corresponding pages,
> > so the redundant pte_devmap case can be removed.
> > 
> > [1] https://patchwork.kernel.org/patch/11494691/
> 
> This is a terrible changelog.  What it should say is something like:
> 
> Since commit 25b2995a35b6 ("mm: remove MEMORY_DEVICE_PUBLIC support"),
> the assignment to 'page' has been unnecessary.  Remove it.
> 
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

With this changelog, feel free to tadd
Acked-by: Michal Hocko <mhocko@suse.com>

> 
> > Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
> > ---
> >  mm/memory.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/mm/memory.c b/mm/memory.c
> > index f703fe8..2143827 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -802,8 +802,6 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
> >  		get_page(page);
> >  		page_dup_rmap(page, false);
> >  		rss[mm_counter(page)]++;
> > -	} else if (pte_devmap(pte)) {
> > -		page = pte_page(pte);
> >  	}
> >  
> >  out_set_pte:
> > -- 
> > 1.9.1
> > 
> > 

-- 
Michal Hocko
SUSE Labs