diff mbox series

[RFC,1/2] mm/khugepaged: Convert __collapse_huge_page_isolate() to use folios

Message ID 20230922193639.10158-2-vishal.moola@gmail.com (mailing list archive)
State New
Headers show
Series Remove compound_pagelist from khugepaged | expand

Commit Message

Vishal Moola Sept. 22, 2023, 7:36 p.m. UTC
This is in preparation for the removal of the khugepaged compound_pagelist.

Replaces 11 calls to compound_head() with 1, and removes 499 bytes of
kernel text.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/khugepaged.c | 52 ++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

Comments

Matthew Wilcox Sept. 22, 2023, 7:46 p.m. UTC | #1
On Fri, Sep 22, 2023 at 12:36:38PM -0700, Vishal Moola (Oracle) wrote:
> This is in preparation for the removal of the khugepaged compound_pagelist.
> 
> Replaces 11 calls to compound_head() with 1, and removes 499 bytes of
> kernel text.

Looks good to me.

> @@ -634,32 +633,33 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>  		 * Isolate the page to avoid collapsing an hugepage
>  		 * currently in use by the VM.
>  		 */
> -		if (!isolate_lru_page(page)) {
> -			unlock_page(page);
> +		if (!folio_isolate_lru(folio)) {
> +			folio_unlock(folio);
>  			result = SCAN_DEL_PAGE_LRU;
>  			goto out;
>  		}
> -		mod_node_page_state(page_pgdat(page),
> -				NR_ISOLATED_ANON + page_is_file_lru(page),
> -				compound_nr(page));
> -		VM_BUG_ON_PAGE(!PageLocked(page), page);
> -		VM_BUG_ON_PAGE(PageLRU(page), page);
> +		node_stat_mod_folio(folio,
> +				NR_ISOLATED_ANON + folio_is_file_lru(folio),
> +				folio_nr_pages(folio));
> +		VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> +		VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
>  
> -		if (PageCompound(page))
> -			list_add_tail(&page->lru, compound_pagelist);
> +		if (folio_test_large(folio))
> +			list_add_tail(&folio->lru, compound_pagelist);
>  next:
>  		/*
>  		 * If collapse was initiated by khugepaged, check that there is
>  		 * enough young pte to justify collapsing the page
>  		 */
>  		if (cc->is_khugepaged &&
> -		    (pte_young(pteval) || page_is_young(page) ||
> -		     PageReferenced(page) || mmu_notifier_test_young(vma->vm_mm,
> +		    (pte_young(pteval) || folio_test_young(folio) ||
> +		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
>  								     address)))
>  			referenced++;
>  
>  		if (pte_write(pteval))
>  			writable = true;
> +

Spurious change here.  Other than that,

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
David Hildenbrand Sept. 25, 2023, 7:18 a.m. UTC | #2
On 22.09.23 21:36, Vishal Moola (Oracle) wrote:
> This is in preparation for the removal of the khugepaged compound_pagelist.
> 
> Replaces 11 calls to compound_head() with 1, and removes 499 bytes of
> kernel text.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>   mm/khugepaged.c | 52 ++++++++++++++++++++++++-------------------------
>   1 file changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 88433cc25d8a..f46a7a7c489f 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -541,7 +541,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>   					struct collapse_control *cc,
>   					struct list_head *compound_pagelist)
>   {
> -	struct page *page = NULL;
> +	struct folio *folio = NULL;
>   	pte_t *_pte;
>   	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
>   	bool writable = false;
> @@ -570,15 +570,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>   			result = SCAN_PTE_UFFD_WP;
>   			goto out;
>   		}
> -		page = vm_normal_page(vma, address, pteval);
> -		if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
> +		folio = vm_normal_folio(vma, address, pteval);
> +		if (unlikely(!folio) || unlikely(folio_is_zone_device(folio))) {
>   			result = SCAN_PAGE_NULL;
>   			goto out;
>   		}
>   
> -		VM_BUG_ON_PAGE(!PageAnon(page), page);
> +		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
>   
> -		if (page_mapcount(page) > 1) {
> +		if (folio_estimated_sharers(folio) > 1) {

For a tail page of a PTE-mapped THP this is not the same.

The possible side effects this might have should be spelled out in the 
patch description.
Vishal Moola Sept. 25, 2023, 6:57 p.m. UTC | #3
On Mon, Sep 25, 2023 at 12:18 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 22.09.23 21:36, Vishal Moola (Oracle) wrote:
> > This is in preparation for the removal of the khugepaged compound_pagelist.
> >
> > Replaces 11 calls to compound_head() with 1, and removes 499 bytes of
> > kernel text.
> >
> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > ---
> >   mm/khugepaged.c | 52 ++++++++++++++++++++++++-------------------------
> >   1 file changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 88433cc25d8a..f46a7a7c489f 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -541,7 +541,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> >                                       struct collapse_control *cc,
> >                                       struct list_head *compound_pagelist)
> >   {
> > -     struct page *page = NULL;
> > +     struct folio *folio = NULL;
> >       pte_t *_pte;
> >       int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
> >       bool writable = false;
> > @@ -570,15 +570,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> >                       result = SCAN_PTE_UFFD_WP;
> >                       goto out;
> >               }
> > -             page = vm_normal_page(vma, address, pteval);
> > -             if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
> > +             folio = vm_normal_folio(vma, address, pteval);
> > +             if (unlikely(!folio) || unlikely(folio_is_zone_device(folio))) {
> >                       result = SCAN_PAGE_NULL;
> >                       goto out;
> >               }
> >
> > -             VM_BUG_ON_PAGE(!PageAnon(page), page);
> > +             VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
> >
> > -             if (page_mapcount(page) > 1) {
> > +             if (folio_estimated_sharers(folio) > 1) {
>
> For a tail page of a PTE-mapped THP this is not the same.
>
> The possible side effects this might have should be spelled out in the
> patch description.

Ok, I'll add the side effects to the commit message in v2.
diff mbox series

Patch

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 88433cc25d8a..f46a7a7c489f 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -541,7 +541,7 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 					struct collapse_control *cc,
 					struct list_head *compound_pagelist)
 {
-	struct page *page = NULL;
+	struct folio *folio = NULL;
 	pte_t *_pte;
 	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
 	bool writable = false;
@@ -570,15 +570,15 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 			result = SCAN_PTE_UFFD_WP;
 			goto out;
 		}
-		page = vm_normal_page(vma, address, pteval);
-		if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
+		folio = vm_normal_folio(vma, address, pteval);
+		if (unlikely(!folio) || unlikely(folio_is_zone_device(folio))) {
 			result = SCAN_PAGE_NULL;
 			goto out;
 		}
 
-		VM_BUG_ON_PAGE(!PageAnon(page), page);
+		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
 
-		if (page_mapcount(page) > 1) {
+		if (folio_estimated_sharers(folio) > 1) {
 			++shared;
 			if (cc->is_khugepaged &&
 			    shared > khugepaged_max_ptes_shared) {
@@ -588,16 +588,15 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 			}
 		}
 
-		if (PageCompound(page)) {
-			struct page *p;
-			page = compound_head(page);
+		if (folio_test_large(folio)) {
+			struct folio *f;
 
 			/*
 			 * Check if we have dealt with the compound page
 			 * already
 			 */
-			list_for_each_entry(p, compound_pagelist, lru) {
-				if (page == p)
+			list_for_each_entry(f, compound_pagelist, lru) {
+				if (folio == f)
 					goto next;
 			}
 		}
@@ -608,7 +607,7 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		 * is needed to serialize against split_huge_page
 		 * when invoked from the VM.
 		 */
-		if (!trylock_page(page)) {
+		if (!folio_trylock(folio)) {
 			result = SCAN_PAGE_LOCK;
 			goto out;
 		}
@@ -624,8 +623,8 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		 * but not from this process. The other process cannot write to
 		 * the page, only trigger CoW.
 		 */
-		if (!is_refcount_suitable(page)) {
-			unlock_page(page);
+		if (!is_refcount_suitable(&folio->page)) {
+			folio_unlock(folio);
 			result = SCAN_PAGE_COUNT;
 			goto out;
 		}
@@ -634,32 +633,33 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		 * Isolate the page to avoid collapsing an hugepage
 		 * currently in use by the VM.
 		 */
-		if (!isolate_lru_page(page)) {
-			unlock_page(page);
+		if (!folio_isolate_lru(folio)) {
+			folio_unlock(folio);
 			result = SCAN_DEL_PAGE_LRU;
 			goto out;
 		}
-		mod_node_page_state(page_pgdat(page),
-				NR_ISOLATED_ANON + page_is_file_lru(page),
-				compound_nr(page));
-		VM_BUG_ON_PAGE(!PageLocked(page), page);
-		VM_BUG_ON_PAGE(PageLRU(page), page);
+		node_stat_mod_folio(folio,
+				NR_ISOLATED_ANON + folio_is_file_lru(folio),
+				folio_nr_pages(folio));
+		VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
+		VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
 
-		if (PageCompound(page))
-			list_add_tail(&page->lru, compound_pagelist);
+		if (folio_test_large(folio))
+			list_add_tail(&folio->lru, compound_pagelist);
 next:
 		/*
 		 * If collapse was initiated by khugepaged, check that there is
 		 * enough young pte to justify collapsing the page
 		 */
 		if (cc->is_khugepaged &&
-		    (pte_young(pteval) || page_is_young(page) ||
-		     PageReferenced(page) || mmu_notifier_test_young(vma->vm_mm,
+		    (pte_young(pteval) || folio_test_young(folio) ||
+		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
 								     address)))
 			referenced++;
 
 		if (pte_write(pteval))
 			writable = true;
+
 	}
 
 	if (unlikely(!writable)) {
@@ -668,13 +668,13 @@  static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		result = SCAN_LACK_REFERENCED_PAGE;
 	} else {
 		result = SCAN_SUCCEED;
-		trace_mm_collapse_huge_page_isolate(page, none_or_zero,
+		trace_mm_collapse_huge_page_isolate(&folio->page, none_or_zero,
 						    referenced, writable, result);
 		return result;
 	}
 out:
 	release_pte_pages(pte, _pte, compound_pagelist);
-	trace_mm_collapse_huge_page_isolate(page, none_or_zero,
+	trace_mm_collapse_huge_page_isolate(&folio->page, none_or_zero,
 					    referenced, writable, result);
 	return result;
 }