diff mbox series

[1/9] mm: Convert ksm_might_need_to_copy() to work on folios

Message ID 20231211162214.2146080-2-willy@infradead.org (mailing list archive)
State New
Headers show
Series Finish two folio conversions | expand

Commit Message

Matthew Wilcox Dec. 11, 2023, 4:22 p.m. UTC 
Accept a folio as an argument and return a folio result.  Removes
a call to compound_head() in do_swap_page(), and prevents folio &
page from getting out of sync in unuse_pte().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/ksm.h |  6 +++---
 mm/ksm.c            | 21 +++++++++++----------
 mm/memory.c         |  8 ++++----
 mm/swapfile.c       |  8 +++++---
 4 files changed, 23 insertions(+), 20 deletions(-)

Comments

David Hildenbrand Dec. 12, 2023, 12:32 p.m. UTC | #1 
> +++ b/mm/swapfile.c
> @@ -1749,11 +1749,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
>   	int ret = 1;
>   
>   	swapcache = page;
> -	page = ksm_might_need_to_copy(page, vma, addr);
> -	if (unlikely(!page))
> +	folio = ksm_might_need_to_copy(folio, vma, addr);
> +	if (unlikely(!folio))
>   		return -ENOMEM;
> -	else if (unlikely(PTR_ERR(page) == -EHWPOISON))
> +	else if (unlikely(folio == ERR_PTR(-EHWPOISON)))
>   		hwpoisoned = true;
> +	else
> +		page = folio_file_page(folio, swp_offset(entry));

Just to double-check:

assuming ksm_might_need_to_copy() in fact allocated a fresh (order-0) 
folio, folio_file_page() will simply translate to "folio_page(folio, 0)" 
and should be fine.
Matthew Wilcox Dec. 12, 2023, 12:43 p.m. UTC | #2 
On Tue, Dec 12, 2023 at 01:32:50PM +0100, David Hildenbrand wrote:
> > +++ b/mm/swapfile.c
> > @@ -1749,11 +1749,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
> >   	int ret = 1;
> >   	swapcache = page;
> > -	page = ksm_might_need_to_copy(page, vma, addr);
> > -	if (unlikely(!page))
> > +	folio = ksm_might_need_to_copy(folio, vma, addr);
> > +	if (unlikely(!folio))
> >   		return -ENOMEM;
> > -	else if (unlikely(PTR_ERR(page) == -EHWPOISON))
> > +	else if (unlikely(folio == ERR_PTR(-EHWPOISON)))
> >   		hwpoisoned = true;
> > +	else
> > +		page = folio_file_page(folio, swp_offset(entry));
> 
> Just to double-check:
> 
> assuming ksm_might_need_to_copy() in fact allocated a fresh (order-0) folio,
> folio_file_page() will simply translate to "folio_page(folio, 0)" and should
> be fine.

That's right:

static inline struct page *folio_file_page(struct folio *folio, pgoff_t index)
{
        return folio_page(folio, index & (folio_nr_pages(folio) - 1));
}

so an order-0 folio will return the only page in the folio.
David Hildenbrand Dec. 12, 2023, 12:43 p.m. UTC | #3 
On 12.12.23 13:43, Matthew Wilcox wrote:
> On Tue, Dec 12, 2023 at 01:32:50PM +0100, David Hildenbrand wrote:
>>> +++ b/mm/swapfile.c
>>> @@ -1749,11 +1749,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
>>>    	int ret = 1;
>>>    	swapcache = page;
>>> -	page = ksm_might_need_to_copy(page, vma, addr);
>>> -	if (unlikely(!page))
>>> +	folio = ksm_might_need_to_copy(folio, vma, addr);
>>> +	if (unlikely(!folio))
>>>    		return -ENOMEM;
>>> -	else if (unlikely(PTR_ERR(page) == -EHWPOISON))
>>> +	else if (unlikely(folio == ERR_PTR(-EHWPOISON)))
>>>    		hwpoisoned = true;
>>> +	else
>>> +		page = folio_file_page(folio, swp_offset(entry));
>>
>> Just to double-check:
>>
>> assuming ksm_might_need_to_copy() in fact allocated a fresh (order-0) folio,
>> folio_file_page() will simply translate to "folio_page(folio, 0)" and should
>> be fine.
> 
> That's right:
> 
> static inline struct page *folio_file_page(struct folio *folio, pgoff_t index)
> {
>          return folio_page(folio, index & (folio_nr_pages(folio) - 1));
> }
> 
> so an order-0 folio will return the only page in the folio.
> 

LGTM then

Reviewed-by: David Hildenbrand <david@redhat.com>
diff mbox series

Patch

diff --git a/include/linux/ksm.h b/include/linux/ksm.h
index 4643d5244e77..401348e9f92b 100644
--- a/include/linux/ksm.h
+++ b/include/linux/ksm.h
@@ -76,7 +76,7 @@  static inline void ksm_exit(struct mm_struct *mm)
  * We'd like to make this conditional on vma->vm_flags & VM_MERGEABLE,
  * but what if the vma was unmerged while the page was swapped out?
  */
-struct page *ksm_might_need_to_copy(struct page *page,
+struct folio *ksm_might_need_to_copy(struct folio *folio,
 			struct vm_area_struct *vma, unsigned long addr);
 
 void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc);
@@ -129,10 +129,10 @@  static inline int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 	return 0;
 }
 
-static inline struct page *ksm_might_need_to_copy(struct page *page,
+static inline struct folio *ksm_might_need_to_copy(struct folio *folio,
 			struct vm_area_struct *vma, unsigned long addr)
 {
-	return page;
+	return folio;
 }
 
 static inline void rmap_walk_ksm(struct folio *folio,
diff --git a/mm/ksm.c b/mm/ksm.c
index b93389a3780e..16532fa85a46 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2875,30 +2875,30 @@  void __ksm_exit(struct mm_struct *mm)
 	trace_ksm_exit(mm);
 }
 
-struct page *ksm_might_need_to_copy(struct page *page,
+struct folio *ksm_might_need_to_copy(struct folio *folio,
 			struct vm_area_struct *vma, unsigned long addr)
 {
-	struct folio *folio = page_folio(page);
+	struct page *page = folio_page(folio, 0);
 	struct anon_vma *anon_vma = folio_anon_vma(folio);
 	struct folio *new_folio;
 
 	if (folio_test_large(folio))
-		return page;
+		return folio;
 
 	if (folio_test_ksm(folio)) {
 		if (folio_stable_node(folio) &&
 		    !(ksm_run & KSM_RUN_UNMERGE))
-			return page;	/* no need to copy it */
+			return folio;	/* no need to copy it */
 	} else if (!anon_vma) {
-		return page;		/* no need to copy it */
+		return folio;		/* no need to copy it */
 	} else if (folio->index == linear_page_index(vma, addr) &&
 			anon_vma->root == vma->anon_vma->root) {
-		return page;		/* still no need to copy it */
+		return folio;		/* still no need to copy it */
 	}
 	if (PageHWPoison(page))
 		return ERR_PTR(-EHWPOISON);
 	if (!folio_test_uptodate(folio))
-		return page;		/* let do_swap_page report the error */
+		return folio;		/* let do_swap_page report the error */
 
 	new_folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, addr, false);
 	if (new_folio &&
@@ -2907,9 +2907,10 @@  struct page *ksm_might_need_to_copy(struct page *page,
 		new_folio = NULL;
 	}
 	if (new_folio) {
-		if (copy_mc_user_highpage(&new_folio->page, page, addr, vma)) {
+		if (copy_mc_user_highpage(folio_page(new_folio, 0), page,
+								addr, vma)) {
 			folio_put(new_folio);
-			memory_failure_queue(page_to_pfn(page), 0);
+			memory_failure_queue(folio_pfn(folio), 0);
 			return ERR_PTR(-EHWPOISON);
 		}
 		folio_set_dirty(new_folio);
@@ -2920,7 +2921,7 @@  struct page *ksm_might_need_to_copy(struct page *page,
 #endif
 	}
 
-	return new_folio ? &new_folio->page : NULL;
+	return new_folio;
 }
 
 void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
diff --git a/mm/memory.c b/mm/memory.c
index 055647120f01..318f923134e4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3942,15 +3942,15 @@  vm_fault_t do_swap_page(struct vm_fault *vmf)
 		 * page->index of !PageKSM() pages would be nonlinear inside the
 		 * anon VMA -- PageKSM() is lost on actual swapout.
 		 */
-		page = ksm_might_need_to_copy(page, vma, vmf->address);
-		if (unlikely(!page)) {
+		folio = ksm_might_need_to_copy(folio, vma, vmf->address);
+		if (unlikely(!folio)) {
 			ret = VM_FAULT_OOM;
 			goto out_page;
-		} else if (unlikely(PTR_ERR(page) == -EHWPOISON)) {
+		} else if (unlikely(folio == ERR_PTR(-EHWPOISON))) {
 			ret = VM_FAULT_HWPOISON;
 			goto out_page;
 		}
-		folio = page_folio(page);
+		page = folio_page(folio, 0);
 
 		/*
 		 * If we want to map a page that's in the swapcache writable, we
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 8be70912e298..0371b7b3cd27 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1749,11 +1749,13 @@  static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
 	int ret = 1;
 
 	swapcache = page;
-	page = ksm_might_need_to_copy(page, vma, addr);
-	if (unlikely(!page))
+	folio = ksm_might_need_to_copy(folio, vma, addr);
+	if (unlikely(!folio))
 		return -ENOMEM;
-	else if (unlikely(PTR_ERR(page) == -EHWPOISON))
+	else if (unlikely(folio == ERR_PTR(-EHWPOISON)))
 		hwpoisoned = true;
+	else
+		page = folio_file_page(folio, swp_offset(entry));
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	if (unlikely(!pte || !pte_same_as_swp(ptep_get(pte),