[PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[jglisse]

From: Jérôme Glisse <jglisse@redhat.com>

MMU notifiers can sleep, but in try_to_unmap_one() we call
mmu_notifier_invalidate_page() under page table lock.

Let's instead use mmu_notifier_invalidate_range() outside
page_vma_mapped_walk() loop.

Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
---
 mm/rmap.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index aff607d5f7d2..d60e887f1cda 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	};
 	pte_t pteval;
 	struct page *subpage;
-	bool ret = true;
+	bool ret = true, invalidation_needed = false;
+	unsigned long end = address + PAGE_SIZE;
 	enum ttu_flags flags = (enum ttu_flags)arg;
 
 	/* munlock has nothing to gain from examining un-locked vmas */
@@ -1386,7 +1387,6 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		VM_BUG_ON_PAGE(!pvmw.pte, page);
 
 		subpage = page - page_to_pfn(page) + pte_pfn(*pvmw.pte);
-		address = pvmw.address;
 
 		if (IS_ENABLED(CONFIG_MIGRATION) &&
 		    (flags & TTU_MIGRATION) &&
@@ -1394,7 +1394,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_entry_t entry;
 			pte_t swp_pte;
 
-			pteval = ptep_get_and_clear(mm, address, pvmw.pte);
+			pteval = ptep_get_and_clear(mm, pvmw.address,
+						    pvmw.pte);
 
 			/*
 			 * Store the pfn of the page in a special migration
@@ -1405,12 +1406,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 			goto discard;
 		}
 
 		if (!(flags & TTU_IGNORE_ACCESS)) {
-			if (ptep_clear_flush_young_notify(vma, address,
+			if (ptep_clear_flush_young_notify(vma, pvmw.address,
 						pvmw.pte)) {
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
@@ -1419,7 +1420,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		}
 
 		/* Nuke the page table entry. */
-		flush_cache_page(vma, address, pte_pfn(*pvmw.pte));
+		flush_cache_page(vma, pvmw.address, pte_pfn(*pvmw.pte));
 		if (should_defer_flush(mm, flags)) {
 			/*
 			 * We clear the PTE but do not flush so potentially
@@ -1429,11 +1430,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			 * transition on a cached TLB entry is written through
 			 * and traps if the PTE is unmapped.
 			 */
-			pteval = ptep_get_and_clear(mm, address, pvmw.pte);
+			pteval = ptep_get_and_clear(mm, pvmw.address,
+						    pvmw.pte);
 
 			set_tlb_ubc_flush_pending(mm, pte_dirty(pteval));
 		} else {
-			pteval = ptep_clear_flush(vma, address, pvmw.pte);
+			pteval = ptep_clear_flush(vma, pvmw.address, pvmw.pte);
 		}
 
 		/* Move the dirty bit to the page. Now the pte is gone. */
@@ -1448,12 +1450,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			if (PageHuge(page)) {
 				int nr = 1 << compound_order(page);
 				hugetlb_count_sub(nr, mm);
-				set_huge_swap_pte_at(mm, address,
+				set_huge_swap_pte_at(mm, pvmw.address,
 						     pvmw.pte, pteval,
 						     vma_mmu_pagesize(vma));
 			} else {
 				dec_mm_counter(mm, mm_counter(page));
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 			}
 
 		} else if (pte_unused(pteval)) {
@@ -1477,7 +1479,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 		} else if (PageAnon(page)) {
 			swp_entry_t entry = { .val = page_private(subpage) };
 			pte_t swp_pte;
@@ -1503,7 +1505,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 				 * If the page was redirtied, it cannot be
 				 * discarded. Remap the page to page table.
 				 */
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 				SetPageSwapBacked(page);
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
@@ -1511,7 +1513,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			}
 
 			if (swap_duplicate(entry) < 0) {
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
 				break;
@@ -1527,14 +1529,18 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 		} else
 			dec_mm_counter(mm, mm_counter_file(page));
 discard:
 		page_remove_rmap(subpage, PageHuge(page));
 		put_page(page);
-		mmu_notifier_invalidate_page(mm, address);
+		end = pvmw.address + PAGE_SIZE;
+		invalidation_needed = true;
 	}
+
+	if (invalidation_needed)
+		mmu_notifier_invalidate_range(mm, address, end);
 	return ret;
 }
 
-- 
2.13.4

[PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[jglisse]

From: JA(C)rA'me Glisse <jglisse@redhat.com>

MMU notifiers can sleep, but in try_to_unmap_one() we call
mmu_notifier_invalidate_page() under page table lock.

Let's instead use mmu_notifier_invalidate_range() outside
page_vma_mapped_walk() loop.

Signed-off-by: JA(C)rA'me Glisse <jglisse@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
---
 mm/rmap.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index aff607d5f7d2..d60e887f1cda 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	};
 	pte_t pteval;
 	struct page *subpage;
-	bool ret = true;
+	bool ret = true, invalidation_needed = false;
+	unsigned long end = address + PAGE_SIZE;
 	enum ttu_flags flags = (enum ttu_flags)arg;
 
 	/* munlock has nothing to gain from examining un-locked vmas */
@@ -1386,7 +1387,6 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		VM_BUG_ON_PAGE(!pvmw.pte, page);
 
 		subpage = page - page_to_pfn(page) + pte_pfn(*pvmw.pte);
-		address = pvmw.address;
 
 		if (IS_ENABLED(CONFIG_MIGRATION) &&
 		    (flags & TTU_MIGRATION) &&
@@ -1394,7 +1394,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_entry_t entry;
 			pte_t swp_pte;
 
-			pteval = ptep_get_and_clear(mm, address, pvmw.pte);
+			pteval = ptep_get_and_clear(mm, pvmw.address,
+						    pvmw.pte);
 
 			/*
 			 * Store the pfn of the page in a special migration
@@ -1405,12 +1406,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 			goto discard;
 		}
 
 		if (!(flags & TTU_IGNORE_ACCESS)) {
-			if (ptep_clear_flush_young_notify(vma, address,
+			if (ptep_clear_flush_young_notify(vma, pvmw.address,
 						pvmw.pte)) {
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
@@ -1419,7 +1420,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		}
 
 		/* Nuke the page table entry. */
-		flush_cache_page(vma, address, pte_pfn(*pvmw.pte));
+		flush_cache_page(vma, pvmw.address, pte_pfn(*pvmw.pte));
 		if (should_defer_flush(mm, flags)) {
 			/*
 			 * We clear the PTE but do not flush so potentially
@@ -1429,11 +1430,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			 * transition on a cached TLB entry is written through
 			 * and traps if the PTE is unmapped.
 			 */
-			pteval = ptep_get_and_clear(mm, address, pvmw.pte);
+			pteval = ptep_get_and_clear(mm, pvmw.address,
+						    pvmw.pte);
 
 			set_tlb_ubc_flush_pending(mm, pte_dirty(pteval));
 		} else {
-			pteval = ptep_clear_flush(vma, address, pvmw.pte);
+			pteval = ptep_clear_flush(vma, pvmw.address, pvmw.pte);
 		}
 
 		/* Move the dirty bit to the page. Now the pte is gone. */
@@ -1448,12 +1450,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			if (PageHuge(page)) {
 				int nr = 1 << compound_order(page);
 				hugetlb_count_sub(nr, mm);
-				set_huge_swap_pte_at(mm, address,
+				set_huge_swap_pte_at(mm, pvmw.address,
 						     pvmw.pte, pteval,
 						     vma_mmu_pagesize(vma));
 			} else {
 				dec_mm_counter(mm, mm_counter(page));
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 			}
 
 		} else if (pte_unused(pteval)) {
@@ -1477,7 +1479,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 		} else if (PageAnon(page)) {
 			swp_entry_t entry = { .val = page_private(subpage) };
 			pte_t swp_pte;
@@ -1503,7 +1505,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 				 * If the page was redirtied, it cannot be
 				 * discarded. Remap the page to page table.
 				 */
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 				SetPageSwapBacked(page);
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
@@ -1511,7 +1513,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			}
 
 			if (swap_duplicate(entry) < 0) {
-				set_pte_at(mm, address, pvmw.pte, pteval);
+				set_pte_at(mm, pvmw.address, pvmw.pte, pteval);
 				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
 				break;
@@ -1527,14 +1529,18 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pteval))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
 		} else
 			dec_mm_counter(mm, mm_counter_file(page));
 discard:
 		page_remove_rmap(subpage, PageHuge(page));
 		put_page(page);
-		mmu_notifier_invalidate_page(mm, address);
+		end = pvmw.address + PAGE_SIZE;
+		invalidation_needed = true;
 	}
+
+	if (invalidation_needed)
+		mmu_notifier_invalidate_range(mm, address, end);
 	return ret;
 }
 
-- 
2.13.4

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

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Kirill A. Shutemov]

On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
> 
> MMU notifiers can sleep, but in try_to_unmap_one() we call
> mmu_notifier_invalidate_page() under page table lock.
> 
> Let's instead use mmu_notifier_invalidate_range() outside
> page_vma_mapped_walk() loop.
> 
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
> ---
>  mm/rmap.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index aff607d5f7d2..d60e887f1cda 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  	};
>  	pte_t pteval;
>  	struct page *subpage;
> -	bool ret = true;
> +	bool ret = true, invalidation_needed = false;
> +	unsigned long end = address + PAGE_SIZE;

I think it should be 'address + (1UL << compound_order(page))'.

-- 
 Kirill A. Shutemov

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Kirill A. Shutemov]

On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
> 
> MMU notifiers can sleep, but in try_to_unmap_one() we call
> mmu_notifier_invalidate_page() under page table lock.
> 
> Let's instead use mmu_notifier_invalidate_range() outside
> page_vma_mapped_walk() loop.
> 
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()")
> ---
>  mm/rmap.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index aff607d5f7d2..d60e887f1cda 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  	};
>  	pte_t pteval;
>  	struct page *subpage;
> -	bool ret = true;
> +	bool ret = true, invalidation_needed = false;
> +	unsigned long end = address + PAGE_SIZE;

I think it should be 'address + (1UL << compound_order(page))'.

-- 
 Kirill A. Shutemov

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

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Jerome Glisse]

> On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > From: Jérôme Glisse <jglisse@redhat.com>
> > 
> > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > mmu_notifier_invalidate_page() under page table lock.
> > 
> > Let's instead use mmu_notifier_invalidate_range() outside
> > page_vma_mapped_walk() loop.
> > 
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > page_vma_mapped_walk()")
> > ---
> >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> >  1 file changed, 21 insertions(+), 15 deletions(-)
> > 
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index aff607d5f7d2..d60e887f1cda 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > struct vm_area_struct *vma,
> >  	};
> >  	pte_t pteval;
> >  	struct page *subpage;
> > -	bool ret = true;
> > +	bool ret = true, invalidation_needed = false;
> > +	unsigned long end = address + PAGE_SIZE;
> 
> I think it should be 'address + (1UL << compound_order(page))'.

Can't address point to something else than first page in huge page ?
Also i did use end as an optimization ie maybe not all the pte in the
range are valid and thus they not all need to be invalidated hence by
tracking the last one that needs invalidation i am limiting the range.

But it is a small optimization so i am not attach to it.

Jérôme

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Jerome Glisse]

> On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > From: Jérôme Glisse <jglisse@redhat.com>
> > 
> > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > mmu_notifier_invalidate_page() under page table lock.
> > 
> > Let's instead use mmu_notifier_invalidate_range() outside
> > page_vma_mapped_walk() loop.
> > 
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > page_vma_mapped_walk()")
> > ---
> >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> >  1 file changed, 21 insertions(+), 15 deletions(-)
> > 
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index aff607d5f7d2..d60e887f1cda 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > struct vm_area_struct *vma,
> >  	};
> >  	pte_t pteval;
> >  	struct page *subpage;
> > -	bool ret = true;
> > +	bool ret = true, invalidation_needed = false;
> > +	unsigned long end = address + PAGE_SIZE;
> 
> I think it should be 'address + (1UL << compound_order(page))'.

Can't address point to something else than first page in huge page ?
Also i did use end as an optimization ie maybe not all the pte in the
range are valid and thus they not all need to be invalidated hence by
tracking the last one that needs invalidation i am limiting the range.

But it is a small optimization so i am not attach to it.

Jérôme

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

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Andrew Morton]

On Wed, 9 Aug 2017 12:52:46 -0400 (EDT) Jerome Glisse <jglisse@redhat.com> wrote:

> > On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > > From: J__r__me Glisse <jglisse@redhat.com>
> > > 
> > > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > > mmu_notifier_invalidate_page() under page table lock.
> > > 
> > > Let's instead use mmu_notifier_invalidate_range() outside
> > > page_vma_mapped_walk() loop.
> > > 
> > > Signed-off-by: J__r__me Glisse <jglisse@redhat.com>
> > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > > page_vma_mapped_walk()")
> > > ---
> > >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> > >  1 file changed, 21 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index aff607d5f7d2..d60e887f1cda 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > > struct vm_area_struct *vma,
> > >  	};
> > >  	pte_t pteval;
> > >  	struct page *subpage;
> > > -	bool ret = true;
> > > +	bool ret = true, invalidation_needed = false;
> > > +	unsigned long end = address + PAGE_SIZE;
> > 
> > I think it should be 'address + (1UL << compound_order(page))'.
> 
> Can't address point to something else than first page in huge page ?
> Also i did use end as an optimization ie maybe not all the pte in the
> range are valid and thus they not all need to be invalidated hence by
> tracking the last one that needs invalidation i am limiting the range.
> 
> But it is a small optimization so i am not attach to it.
> 

So we need this patch in addition to Kirrill's "rmap: do not call
mmu_notifier_invalidate_page() under ptl". 

Jerome, I'm seeing a bunch of rejects applying this patch to current
mainline.  It's unclear which kernel you're patching but we'll need
something which can go into Linus soon and which is backportable (with
mimimal fixups) into -stable kernels, please.

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Andrew Morton]

On Wed, 9 Aug 2017 12:52:46 -0400 (EDT) Jerome Glisse <jglisse@redhat.com> wrote:

> > On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > > From: J__r__me Glisse <jglisse@redhat.com>
> > > 
> > > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > > mmu_notifier_invalidate_page() under page table lock.
> > > 
> > > Let's instead use mmu_notifier_invalidate_range() outside
> > > page_vma_mapped_walk() loop.
> > > 
> > > Signed-off-by: J__r__me Glisse <jglisse@redhat.com>
> > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > > page_vma_mapped_walk()")
> > > ---
> > >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> > >  1 file changed, 21 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index aff607d5f7d2..d60e887f1cda 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > > struct vm_area_struct *vma,
> > >  	};
> > >  	pte_t pteval;
> > >  	struct page *subpage;
> > > -	bool ret = true;
> > > +	bool ret = true, invalidation_needed = false;
> > > +	unsigned long end = address + PAGE_SIZE;
> > 
> > I think it should be 'address + (1UL << compound_order(page))'.
> 
> Can't address point to something else than first page in huge page ?
> Also i did use end as an optimization ie maybe not all the pte in the
> range are valid and thus they not all need to be invalidated hence by
> tracking the last one that needs invalidation i am limiting the range.
> 
> But it is a small optimization so i am not attach to it.
> 

So we need this patch in addition to Kirrill's "rmap: do not call
mmu_notifier_invalidate_page() under ptl". 

Jerome, I'm seeing a bunch of rejects applying this patch to current
mainline.  It's unclear which kernel you're patching but we'll need
something which can go into Linus soon and which is backportable (with
mimimal fixups) into -stable kernels, please.

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

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Jerome Glisse]

On Wed, Aug 09, 2017 at 01:17:42PM -0700, Andrew Morton wrote:
> On Wed, 9 Aug 2017 12:52:46 -0400 (EDT) Jerome Glisse <jglisse@redhat.com> wrote:
> 
> > > On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > > > From: J__r__me Glisse <jglisse@redhat.com>
> > > > 
> > > > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > > > mmu_notifier_invalidate_page() under page table lock.
> > > > 
> > > > Let's instead use mmu_notifier_invalidate_range() outside
> > > > page_vma_mapped_walk() loop.
> > > > 
> > > > Signed-off-by: J__r__me Glisse <jglisse@redhat.com>
> > > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > > > page_vma_mapped_walk()")
> > > > ---
> > > >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> > > >  1 file changed, 21 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > > index aff607d5f7d2..d60e887f1cda 100644
> > > > --- a/mm/rmap.c
> > > > +++ b/mm/rmap.c
> > > > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > > > struct vm_area_struct *vma,
> > > >  	};
> > > >  	pte_t pteval;
> > > >  	struct page *subpage;
> > > > -	bool ret = true;
> > > > +	bool ret = true, invalidation_needed = false;
> > > > +	unsigned long end = address + PAGE_SIZE;
> > > 
> > > I think it should be 'address + (1UL << compound_order(page))'.
> > 
> > Can't address point to something else than first page in huge page ?
> > Also i did use end as an optimization ie maybe not all the pte in the
> > range are valid and thus they not all need to be invalidated hence by
> > tracking the last one that needs invalidation i am limiting the range.
> > 
> > But it is a small optimization so i am not attach to it.
> > 
> 
> So we need this patch in addition to Kirrill's "rmap: do not call
> mmu_notifier_invalidate_page() under ptl". 

Yes we need both to restore mmu_notifier.

> 
> Jerome, I'm seeing a bunch of rejects applying this patch to current
> mainline.  It's unclear which kernel you're patching but we'll need
> something which can go into Linus soon and which is backportable (with
> mimimal fixups) into -stable kernels, please.
> 

Sorry this was on top of one of my HMM branches. I am reposting with
Kirill end address computation as it is not a big optimization if pte
are already invalid then invalidating them once more should not trigger
any more work.

Jérôme

Re: [PATCH] mm/rmap: try_to_unmap_one() do not call mmu_notifier under ptl

[Jerome Glisse]

On Wed, Aug 09, 2017 at 01:17:42PM -0700, Andrew Morton wrote:
> On Wed, 9 Aug 2017 12:52:46 -0400 (EDT) Jerome Glisse <jglisse@redhat.com> wrote:
> 
> > > On Wed, Aug 09, 2017 at 12:17:09PM -0400, jglisse@redhat.com wrote:
> > > > From: J__r__me Glisse <jglisse@redhat.com>
> > > > 
> > > > MMU notifiers can sleep, but in try_to_unmap_one() we call
> > > > mmu_notifier_invalidate_page() under page table lock.
> > > > 
> > > > Let's instead use mmu_notifier_invalidate_range() outside
> > > > page_vma_mapped_walk() loop.
> > > > 
> > > > Signed-off-by: J__r__me Glisse <jglisse@redhat.com>
> > > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Fixes: c7ab0d2fdc84 ("mm: convert try_to_unmap_one() to use
> > > > page_vma_mapped_walk()")
> > > > ---
> > > >  mm/rmap.c | 36 +++++++++++++++++++++---------------
> > > >  1 file changed, 21 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > > index aff607d5f7d2..d60e887f1cda 100644
> > > > --- a/mm/rmap.c
> > > > +++ b/mm/rmap.c
> > > > @@ -1329,7 +1329,8 @@ static bool try_to_unmap_one(struct page *page,
> > > > struct vm_area_struct *vma,
> > > >  	};
> > > >  	pte_t pteval;
> > > >  	struct page *subpage;
> > > > -	bool ret = true;
> > > > +	bool ret = true, invalidation_needed = false;
> > > > +	unsigned long end = address + PAGE_SIZE;
> > > 
> > > I think it should be 'address + (1UL << compound_order(page))'.
> > 
> > Can't address point to something else than first page in huge page ?
> > Also i did use end as an optimization ie maybe not all the pte in the
> > range are valid and thus they not all need to be invalidated hence by
> > tracking the last one that needs invalidation i am limiting the range.
> > 
> > But it is a small optimization so i am not attach to it.
> > 
> 
> So we need this patch in addition to Kirrill's "rmap: do not call
> mmu_notifier_invalidate_page() under ptl". 

Yes we need both to restore mmu_notifier.

> 
> Jerome, I'm seeing a bunch of rejects applying this patch to current
> mainline.  It's unclear which kernel you're patching but we'll need
> something which can go into Linus soon and which is backportable (with
> mimimal fixups) into -stable kernels, please.
> 

Sorry this was on top of one of my HMM branches. I am reposting with
Kirill end address computation as it is not a big optimization if pte
are already invalid then invalidating them once more should not trigger
any more work.

Jerome

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