diff mbox series

[v3,4/7] mm/swap: introduce swapin_entry for unified readahead policy

Message ID 20240129175423.1987-5-ryncsn@gmail.com (mailing list archive)
State New
Headers show
Series swapin refactor for optimization and unified readahead | expand

Commit Message

Kairui Song Jan. 29, 2024, 5:54 p.m. UTC 
From: Kairui Song <kasong@tencent.com>

Introduce swapin_entry which merges swapin_readahead and swapin_direct
making it the main entry for swapin pages, and use a unified swapin
readahead policy.

This commit makes swapoff make use of this new helper and skip readahead
for SYNCHRONOUS_IO device since it's not helpful here. Now swapping
off a 10G ZRAM (lzo-rle) after same workload is faster since readahead
is skipped and overhead is reduced.

Before:
time swapoff /dev/zram0
real    0m12.337s
user    0m0.001s
sys     0m12.329s

After:
time swapoff /dev/zram0
real    0m9.728s
user    0m0.001s
sys     0m9.719s

Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/memory.c     | 18 +++---------------
 mm/swap.h       | 16 ++++------------
 mm/swap_state.c | 40 ++++++++++++++++++++++++----------------
 mm/swapfile.c   |  7 ++-----
 4 files changed, 33 insertions(+), 48 deletions(-)

Comments

Huang, Ying Jan. 30, 2024, 6:29 a.m. UTC | #1 
Kairui Song <ryncsn@gmail.com> writes:

> From: Kairui Song <kasong@tencent.com>
>
> Introduce swapin_entry which merges swapin_readahead and swapin_direct
> making it the main entry for swapin pages, and use a unified swapin
> readahead policy.
>
> This commit makes swapoff make use of this new helper and skip readahead
> for SYNCHRONOUS_IO device since it's not helpful here. Now swapping
> off a 10G ZRAM (lzo-rle) after same workload is faster since readahead
> is skipped and overhead is reduced.
>
> Before:
> time swapoff /dev/zram0
> real    0m12.337s
> user    0m0.001s
> sys     0m12.329s
>
> After:
> time swapoff /dev/zram0
> real    0m9.728s
> user    0m0.001s
> sys     0m9.719s
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/memory.c     | 18 +++---------------
>  mm/swap.h       | 16 ++++------------
>  mm/swap_state.c | 40 ++++++++++++++++++++++++----------------
>  mm/swapfile.c   |  7 ++-----
>  4 files changed, 33 insertions(+), 48 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 81dc9d467f4e..8711f8a07039 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3864,20 +3864,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  	swapcache = folio;
>  
>  	if (!folio) {
> -		if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
> -		    __swap_count(entry) == 1) {
> -			/* skip swapcache and readahead */
> -			folio = swapin_direct(entry, GFP_HIGHUSER_MOVABLE, vmf);
> -			if (folio)
> -				page = &folio->page;
> -		} else {
> -			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> -						vmf);
> -			if (page)
> -				folio = page_folio(page);
> -			swapcache = folio;
> -		}
> -
> +		folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
> +				     vmf, &swapcache);
>  		if (!folio) {
>  			/*
>  			 * Back out if somebody else faulted in this pte
> @@ -3890,11 +3878,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  				ret = VM_FAULT_OOM;
>  			goto unlock;
>  		}
> -

Change by accident?

>  		/* Had to read the page from swap area: Major fault */
>  		ret = VM_FAULT_MAJOR;
>  		count_vm_event(PGMAJFAULT);
>  		count_memcg_event_mm(vma->vm_mm, PGMAJFAULT);
> +		page = folio_file_page(folio, swp_offset(entry));

Better to move this line just after checking !folio.  This make it a
little easier to associate page to folio.

>  	} else if (PageHWPoison(page)) {
>  		/*
>  		 * hwpoisoned dirty swapcache pages are kept for killing
> diff --git a/mm/swap.h b/mm/swap.h
> index 83eab7b67e77..8f8185d3865c 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -54,10 +54,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_direct(swp_entry_t entry, gfp_t flag,
> -			    struct vm_fault *vmf);
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
> +			   struct vm_fault *vmf, struct folio **swapcached);
>  
>  static inline unsigned int folio_swap_flags(struct folio *folio)
>  {
> @@ -88,14 +86,8 @@ static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
>  	return NULL;
>  }
>  
> -struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
> -			struct vm_fault *vmf)
> -{
> -	return NULL;
> -}
> -
> -static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
> -			struct vm_fault *vmf)
> +static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
> +			struct vm_fault *vmf, struct folio **swapcached)
>  {
>  	return NULL;
>  }
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index a450d09fc0db..5e06b2e140d4 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -870,8 +870,8 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
>   * Returns the struct folio for entry and addr after the swap entry is read
>   * in.
>   */
> -struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
> -			    struct vm_fault *vmf)
> +static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
> +				  struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct folio *folio;
> @@ -908,33 +908,41 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
>  }
>  
>  /**
> - * swapin_readahead - swap in pages in hope we need them soon
> + * swapin_entry - swap in a folio from swap entry
>   * @entry: swap entry of this memory
>   * @gfp_mask: memory allocation flags
>   * @vmf: fault information
> + * @swapcache: set to the swapcache folio if swapcache is used
>   *
>   * Returns the struct page for entry and addr, after queueing swapin.
>   *
> - * It's a main entry function for swap readahead. By the configuration,
> + * It's the main entry function for swap in. 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.
> + * or vma-based(ie, virtual address based on faulty address) readahead,
> + * or skip the readahead(ie, ramdisk based swap device).
>   */
> -struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
> -				struct vm_fault *vmf)
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
> +			   struct vm_fault *vmf, struct folio **swapcache)
>  {
>  	struct mempolicy *mpol;
> -	pgoff_t ilx;
>  	struct folio *folio;
> +	pgoff_t ilx;

ditto.

Otherwise, looks good to me.  Thanks!

Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

>  
> -	mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> -	folio = swap_use_vma_readahead() ?
> -		swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
> -		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> -	mpol_cond_put(mpol);
> +	if (data_race(swp_swap_info(entry)->flags & SWP_SYNCHRONOUS_IO) &&
> +	    __swap_count(entry) == 1) {
> +		folio = swapin_direct(entry, gfp_mask, vmf);
> +	} else {
> +		mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> +		if (swap_use_vma_readahead())
> +			folio = swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf);
> +		else
> +			folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> +		mpol_cond_put(mpol);
> +		if (swapcache)
> +			*swapcache = folio;
> +	}
>  
> -	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 606d95b56304..1cf7e72e19e3 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1867,7 +1867,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,
> @@ -1875,10 +1874,8 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  				.pmd = pmd,
>  			};
>  
> -			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> -						&vmf);
> -			if (page)
> -				folio = page_folio(page);
> +			folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
> +					    &vmf, NULL);
>  		}
>  		if (!folio) {
>  			/*
diff mbox series

Patch

diff --git a/mm/memory.c b/mm/memory.c
index 81dc9d467f4e..8711f8a07039 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3864,20 +3864,8 @@  vm_fault_t do_swap_page(struct vm_fault *vmf)
 	swapcache = folio;
 
 	if (!folio) {
-		if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
-		    __swap_count(entry) == 1) {
-			/* skip swapcache and readahead */
-			folio = swapin_direct(entry, GFP_HIGHUSER_MOVABLE, vmf);
-			if (folio)
-				page = &folio->page;
-		} else {
-			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
-						vmf);
-			if (page)
-				folio = page_folio(page);
-			swapcache = folio;
-		}
-
+		folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
+				     vmf, &swapcache);
 		if (!folio) {
 			/*
 			 * Back out if somebody else faulted in this pte
@@ -3890,11 +3878,11 @@  vm_fault_t do_swap_page(struct vm_fault *vmf)
 				ret = VM_FAULT_OOM;
 			goto unlock;
 		}
-
 		/* Had to read the page from swap area: Major fault */
 		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 83eab7b67e77..8f8185d3865c 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -54,10 +54,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_direct(swp_entry_t entry, gfp_t flag,
-			    struct vm_fault *vmf);
+struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
+			   struct vm_fault *vmf, struct folio **swapcached);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {
@@ -88,14 +86,8 @@  static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
 	return NULL;
 }
 
-struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
-			struct vm_fault *vmf)
-{
-	return NULL;
-}
-
-static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
-			struct vm_fault *vmf)
+static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
+			struct vm_fault *vmf, struct folio **swapcached)
 {
 	return NULL;
 }
diff --git a/mm/swap_state.c b/mm/swap_state.c
index a450d09fc0db..5e06b2e140d4 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -870,8 +870,8 @@  static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * Returns the struct folio for entry and addr after the swap entry is read
  * in.
  */
-struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-			    struct vm_fault *vmf)
+static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
+				  struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct folio *folio;
@@ -908,33 +908,41 @@  struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
 }
 
 /**
- * swapin_readahead - swap in pages in hope we need them soon
+ * swapin_entry - swap in a folio from swap entry
  * @entry: swap entry of this memory
  * @gfp_mask: memory allocation flags
  * @vmf: fault information
+ * @swapcache: set to the swapcache folio if swapcache is used
  *
  * Returns the struct page for entry and addr, after queueing swapin.
  *
- * It's a main entry function for swap readahead. By the configuration,
+ * It's the main entry function for swap in. 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.
+ * or vma-based(ie, virtual address based on faulty address) readahead,
+ * or skip the readahead(ie, ramdisk based swap device).
  */
-struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
-				struct vm_fault *vmf)
+struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
+			   struct vm_fault *vmf, struct folio **swapcache)
 {
 	struct mempolicy *mpol;
-	pgoff_t ilx;
 	struct folio *folio;
+	pgoff_t ilx;
 
-	mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
-	folio = swap_use_vma_readahead() ?
-		swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
-		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
-	mpol_cond_put(mpol);
+	if (data_race(swp_swap_info(entry)->flags & SWP_SYNCHRONOUS_IO) &&
+	    __swap_count(entry) == 1) {
+		folio = swapin_direct(entry, gfp_mask, vmf);
+	} else {
+		mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
+		if (swap_use_vma_readahead())
+			folio = swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf);
+		else
+			folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+		mpol_cond_put(mpol);
+		if (swapcache)
+			*swapcache = folio;
+	}
 
-	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 606d95b56304..1cf7e72e19e3 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1867,7 +1867,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,
@@ -1875,10 +1874,8 @@  static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 				.pmd = pmd,
 			};
 
-			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
-						&vmf);
-			if (page)
-				folio = page_folio(page);
+			folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
+					    &vmf, NULL);
 		}
 		if (!folio) {
 			/*