Received-SPF: pass (google.com: domain of linux-kernel+bounces-119855-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 26 Mar 2024 20:03:25 +0000
From: Matthew Wilcox <willy@infradead.org>
To: Kairui Song <kasong@tencent.com>
Cc: linux-mm@kvack.org, "Huang, Ying" <ying.huang@intel.com>,
	Chris Li <chrisl@kernel.org>, Minchan Kim <minchan@kernel.org>,
	Barry Song <v-songbaohua@oppo.com>,
	Ryan Roberts <ryan.roberts@arm.com>, Yu Zhao <yuzhao@google.com>,
	SeongJae Park <sj@kernel.org>, David Hildenbrand <david@redhat.com>,
	Yosry Ahmed <yosryahmed@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>, Nhat Pham <nphamcs@gmail.com>,
	Chengming Zhou <zhouchengming@bytedance.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 03/10] mm/swap: convert swapin_readahead to return a
 folio
Message-ID: <ZgMqDWc7NrR5SIlP@casper.infradead.org>
References: <20240326185032.72159-1-ryncsn@gmail.com>
 <20240326185032.72159-4-ryncsn@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240326185032.72159-4-ryncsn@gmail.com>

On Wed, Mar 27, 2024 at 02:50:25AM +0800, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> Simplify the caller code logic.
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>

I have this patch in my tree which is basically identical, but I think
has a slightly better commit message.  Feel free to merge the two.

From 0c17eefb4c1ca7733ca765ab2f28ee09c9110ec0 Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Date: Fri, 22 Mar 2024 13:28:33 -0400
Subject: [PATCH] mm: Return the folio from swapin_readahead

The unuse_pte_range() caller only wants the folio while do_swap_page()
wants both the page and the folio.  Since do_swap_page() already has
logic for handling both the folio and the page, move the folio-to-page
logic there.  This also lets us allocate larger folios in the
SWP_SYNCHRONOUS_IO path in future.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/memory.c     | 6 ++----
 mm/swap.h       | 6 +++---
 mm/swap_state.c | 8 +++-----
 mm/swapfile.c   | 5 +----
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 40070ef01867..aedf0ee554d1 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4020,7 +4020,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 			/* skip swapcache */
 			folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
 						vma, vmf->address, false);
-			page = &folio->page;
 			if (folio) {
 				__folio_set_locked(folio);
 				__folio_set_swapbacked(folio);
@@ -4045,10 +4044,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 				folio->private = NULL;
 			}
 		} else {
-			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
+			folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
 						vmf);
-			if (page)
-				folio = page_folio(page);
 			swapcache = folio;
 		}
 
@@ -4069,6 +4066,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		ret = VM_FAULT_MAJOR;
 		count_vm_event(PGMAJFAULT);
 		count_memcg_event_mm(vma->vm_mm, PGMAJFAULT);
+		page = folio_file_page(folio, swp_offset(entry));
 	} else if (PageHWPoison(page)) {
 		/*
 		 * hwpoisoned dirty swapcache pages are kept for killing
diff --git a/mm/swap.h b/mm/swap.h
index fc2f6ade7f80..6661b55b2c75 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -55,8 +55,8 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_flags,
 		bool skip_if_exists);
 struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
 		struct mempolicy *mpol, pgoff_t ilx);
-struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
-			      struct vm_fault *vmf);
+struct folio *swapin_readahead(swp_entry_t entry, gfp_t flag,
+		struct vm_fault *vmf);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {
@@ -87,7 +87,7 @@ static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
 	return NULL;
 }
 
-static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
+static inline struct folio *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
 			struct vm_fault *vmf)
 {
 	return NULL;
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 2deac23633cd..f3c379e93bc6 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -885,13 +885,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * @gfp_mask: memory allocation flags
  * @vmf: fault information
  *
- * Returns the struct page for entry and addr, after queueing swapin.
+ * Returns the struct folio for entry and addr, after queueing swapin.
  *
  * It's a main entry function for swap readahead. By the configuration,
  * it will read ahead blocks by cluster-based(ie, physical disk based)
  * or vma-based(ie, virtual address based on faulty address) readahead.
  */
-struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
+struct folio *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
 				struct vm_fault *vmf)
 {
 	struct mempolicy *mpol;
@@ -904,9 +904,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
 		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
 	mpol_cond_put(mpol);
 
-	if (!folio)
-		return NULL;
-	return folio_file_page(folio, swp_offset(entry));
+	return folio;
 }
 
 #ifdef CONFIG_SYSFS
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 5e6d2304a2a4..c9d041ad8df6 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1883,7 +1883,6 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 
 		folio = swap_cache_get_folio(entry, vma, addr);
 		if (!folio) {
-			struct page *page;
 			struct vm_fault vmf = {
 				.vma = vma,
 				.address = addr,
@@ -1891,10 +1890,8 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 				.pmd = pmd,
 			};
 
-			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
+			folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
 						&vmf);
-			if (page)
-				folio = page_folio(page);
 		}
 		if (!folio) {
 			swp_count = READ_ONCE(si->swap_map[offset]);
-- 
2.43.0


>  mm/memory.c     | 8 +++-----
>  mm/swap.h       | 4 ++--
>  mm/swap_state.c | 6 ++----
>  mm/swapfile.c   | 5 +----
>  4 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index e42fadc25268..dfdb620a9123 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4005,12 +4005,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  			if (PTR_ERR(folio) == -EBUSY)
>  				goto out;
>  			need_clear_cache = true;
> -			page = &folio->page;
>  		} else {
> -			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> -						vmf);
> -			if (page)
> -				folio = page_folio(page);
> +			folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE, vmf);
>  			swapcache = folio;
>  		}
>  
> @@ -4027,6 +4023,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  			goto unlock;
>  		}
>  
> +		page = folio_file_page(folio, swp_offset(entry));
> +
>  		/* Had to read the page from swap area: Major fault */
>  		ret = VM_FAULT_MAJOR;
>  		count_vm_event(PGMAJFAULT);
> diff --git a/mm/swap.h b/mm/swap.h
> index 40e902812cc5..aee134907a70 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -57,7 +57,7 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
>  		struct mempolicy *mpol, pgoff_t ilx);
>  struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
>  			    struct vm_fault *vmf);
> -struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
> +struct folio *swapin_readahead(swp_entry_t entry, gfp_t flag,
>  			      struct vm_fault *vmf);
>  
>  static inline unsigned int folio_swap_flags(struct folio *folio)
> @@ -95,7 +95,7 @@ static inline struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
>  	return NULL;
>  }
>  
> -static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
> +static inline struct folio *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
>  			struct vm_fault *vmf)
>  {
>  	return NULL;
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 0a3fa48b3893..2a9c6bdff5ea 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -951,7 +951,7 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
>   * it will read ahead blocks by cluster-based(ie, physical disk based)
>   * or vma-based(ie, virtual address based on faulty address) readahead.
>   */
> -struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
> +struct folio *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  				struct vm_fault *vmf)
>  {
>  	struct mempolicy *mpol;
> @@ -964,9 +964,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
>  	mpol_cond_put(mpol);
>  
> -	if (!folio)
> -		return NULL;
> -	return folio_file_page(folio, swp_offset(entry));
> +	return folio;
>  }
>  
>  #ifdef CONFIG_SYSFS
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 4919423cce76..4dd894395a0f 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1883,7 +1883,6 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  
>  		folio = swap_cache_get_folio(entry, vma, addr);
>  		if (!folio) {
> -			struct page *page;
>  			struct vm_fault vmf = {
>  				.vma = vma,
>  				.address = addr,
> @@ -1891,10 +1890,8 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  				.pmd = pmd,
>  			};
>  
> -			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> +			folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
>  						&vmf);
> -			if (page)
> -				folio = page_folio(page);
>  		}
>  		if (!folio) {
>  			swp_count = READ_ONCE(si->swap_map[offset]);
> -- 
> 2.43.0
>