swapin refactor for optimization and unified readahead

From: Kairui Song <[email protected]>

No feature change, simply move the routine to a standalone function to be re-used later. The error path handling is copied from the "out_page" label, to make the code change minimized for easier reviewing.

Signed-off-by Kairui Song <[email protected]>

---
 mm/memory.c     | 32 ++++----------------------------
 mm/swap.h       |  8 ++++++++
 mm/swap_state.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 28 deletions(-)

mm/memory.c +4 -28

index a0a50d3754f0..0165c8cad489 100644
@@ -3803,7 +3803,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	swp_entry_t entry;
 	pte_t pte;
 	vm_fault_t ret = 0;
-	void *shadow = NULL;
 
 	if (!pte_unmap_same(vmf))
 		goto out;

@@ -3867,33 +3866,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	if (!folio) {
 		if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
 		    __swap_count(entry) == 1) {
-			/* 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);
-
-				if (mem_cgroup_swapin_charge_folio(folio,
-							vma->vm_mm, GFP_KERNEL,
-							entry)) {
-					ret = VM_FAULT_OOM;
-					goto out_page;
-				}
-				mem_cgroup_swapin_uncharge_swap(entry);
-
-				shadow = get_shadow_from_swap_cache(entry);
-				if (shadow)
-					workingset_refault(folio, shadow);
-
-				folio_add_lru(folio);
-
-				/* To provide entry to swap_read_folio() */
-				folio->swap = entry;
-				swap_read_folio(folio, true, NULL);
-				folio->private = NULL;
-			}
+			/* 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);

mm/swap.h +8 -0

index 758c46ca671e..83eab7b67e77 100644
@@ -56,6 +56,8 @@ 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);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -86,6 +88,12 @@ 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)
 {

mm/swap_state.c +0 -0

index e671266ad772..24cb93ed5081 100644

mm/swap_state.c +6 -0

+ * @entry: swap entry of this memory
+ * @gfp_mask: memory allocation flags
+ * @vmf: fault information
+ *
+ * Returns the struct folio for entry and addr after the swap entry is read
+ * in.

mm/swap_state.c +22 -0

+				vma, vmf->address, false);
+	if (folio) {
+		__folio_set_locked(folio);
+		__folio_set_swapbacked(folio);
+
+		if (mem_cgroup_swapin_charge_folio(folio,
+					vma->vm_mm, GFP_KERNEL,
+					entry)) {
+			folio_unlock(folio);
+			folio_put(folio);
+			return NULL;
+		}
+		mem_cgroup_swapin_uncharge_swap(entry);
+
+		shadow = get_shadow_from_swap_cache(entry);
+		if (shadow)
+			workingset_refault(folio, shadow);
+
+		folio_add_lru(folio);
+
+		/* To provide entry to swap_read_folio() */
+		folio->swap = entry;

Quoted 94 lines · Kairui Song

From: Kairui Song <[email protected]>

No feature change, simply move the routine to a standalone function to be re-used later. The error path handling is copied from the "out_page" label, to make the code change minimized for easier reviewing.

Signed-off-by Kairui Song <[email protected]>

---
 mm/memory.c     | 32 ++++----------------------------
 mm/swap.h       |  8 ++++++++
 mm/swap_state.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 28 deletions(-)

mm/memory.c +4 -28

index a0a50d3754f0..0165c8cad489 100644
@@ -3803,7 +3803,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
      swp_entry_t entry;
      pte_t pte;
      vm_fault_t ret = 0;
-     void *shadow = NULL;
 
      if (!pte_unmap_same(vmf))
              goto out;

@@ -3867,33 +3866,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
      if (!folio) {
              if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
                  __swap_count(entry) == 1) {
-                     /* 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);
-
-                             if (mem_cgroup_swapin_charge_folio(folio,
-                                                     vma->vm_mm, GFP_KERNEL,
-                                                     entry)) {
-                                     ret = VM_FAULT_OOM;
-                                     goto out_page;
-                             }
-                             mem_cgroup_swapin_uncharge_swap(entry);
-
-                             shadow = get_shadow_from_swap_cache(entry);
-                             if (shadow)
-                                     workingset_refault(folio, shadow);
-
-                             folio_add_lru(folio);
-
-                             /* To provide entry to swap_read_folio() */
-                             folio->swap = entry;
-                             swap_read_folio(folio, true, NULL);
-                             folio->private = NULL;
-                     }
+                     /* 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);

mm/swap.h +8 -0

index 758c46ca671e..83eab7b67e77 100644
@@ -56,6 +56,8 @@ 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);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -86,6 +88,12 @@ 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)
 {

mm/swap_state.c +0 -0

index e671266ad772..24cb93ed5081 100644

mm/swap_state.c static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask, · line 865

Quoted 5 lines

mm/swap_state.c +2 -0

@@ -861,6 +861,53 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
      return folio;
 }
 
+/**
+ * swapin_direct - swap in folios skipping swap cache and readahead

swap in a folio ...

Quoted 10 lines

mm/swap_state.c +10 -0

+ * @entry: swap entry of this memory
+ * @gfp_mask: memory allocation flags
+ * @vmf: fault information
+ *
+ * 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)
+{

mm/swap_state.c static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask, · line 881

Quoted 6 lines

mm/swap_state.c +6 -0

+     struct vm_area_struct *vma = vmf->vma;
+     struct folio *folio;
+     void *shadow = NULL;
+
+     /* skip swapcache */
+     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,

You passed gfp_mask in, but don't use it.

--
Best Regards,
Huang, Ying

From: Kairui Song <[email protected]>

When swapping in a page, mem_cgroup_swapin_charge_folio is called for new allocated folio, nothing else is referencing the folio so no need to set the lock bit early. This avoided doing extra unlock checks on the error path.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

mm/swap_state.c +2 -7

index 24cb93ed5081..6130de8d5226 100644
@@ -881,16 +881,15 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
 	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
 				vma, vmf->address, false);
 	if (folio) {
-		__folio_set_locked(folio);
-		__folio_set_swapbacked(folio);
-
-		if (mem_cgroup_swapin_charge_folio(folio,
-					vma->vm_mm, GFP_KERNEL,
-					entry)) {
-			folio_unlock(folio);
+		if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
+						   GFP_KERNEL, entry)) {

Quoted 38 lines · Kairui Song

From: Kairui Song <[email protected]>

When swapping in a page, mem_cgroup_swapin_charge_folio is called for new allocated folio, nothing else is referencing the folio so no need to set the lock bit early. This avoided doing extra unlock checks on the error path.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

mm/swap_state.c +6 -7

index 24cb93ed5081..6130de8d5226 100644
@@ -881,16 +881,15 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
      folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
                              vma, vmf->address, false);
      if (folio) {
-             __folio_set_locked(folio);
-             __folio_set_swapbacked(folio);
-
-             if (mem_cgroup_swapin_charge_folio(folio,
-                                     vma->vm_mm, GFP_KERNEL,
-                                     entry)) {
-                     folio_unlock(folio);
+             if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
+                                                GFP_KERNEL, entry)) {
                      folio_put(folio);
                      return NULL;
              }
+
+             __folio_set_locked(folio);
+             __folio_set_swapbacked(folio);
+
              mem_cgroup_swapin_uncharge_swap(entry);
 

mm/swap_state.c struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, · line 895

Quoted 1 line

mm/swap_state.c +0 -0

              shadow = get_shadow_from_swap_cache(entry);

I don't find any issue with the patch. But another caller of mem_cgroup_swapin_charge_folio() in __read_swap_cache_async() setups newly allocated folio in the same way before the change. Better to keep them same? Because the benefit of change is small too.

From: Kairui Song <[email protected]>

Currently, mem_cgroup_swapin_charge_folio is always called with mm argument as NULL, except in swapin_direct.

swapin_direct is used when swapin should skip readahead and swapcache (SWP_SYNCHRONOUS_IO). Other caller paths of mem_cgroup_swapin_charge_folio are for swapin that should not skip readahead and cache.

This could cause swapin charging to behave differently depending on swap device. This currently didn't happen because the only call path of swapin_direct is the direct anon page fault path, where mm equals to current->mm, but will no longer be true if swapin_direct is shared and have other callers (eg, swapoff).

So make swapin_direct also passes NULL for mm, no feature change.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/swap_state.c +0 -0

index 6130de8d5226..d39c5369da21 100644

Quoted 34 lines · Kairui Song

From: Kairui Song <[email protected]>

Currently, mem_cgroup_swapin_charge_folio is always called with mm argument as NULL, except in swapin_direct.

swapin_direct is used when swapin should skip readahead and swapcache (SWP_SYNCHRONOUS_IO). Other caller paths of mem_cgroup_swapin_charge_folio are for swapin that should not skip readahead and cache.

This could cause swapin charging to behave differently depending on swap device. This currently didn't happen because the only call path of swapin_direct is the direct anon page fault path, where mm equals to current->mm, but will no longer be true if swapin_direct is shared and have other callers (eg, swapoff).

So make swapin_direct also passes NULL for mm, no feature change.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/swap_state.c +1 -1

index 6130de8d5226..d39c5369da21 100644
@@ -881,7 +881,7 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
      folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
                              vma, vmf->address, false);
      if (folio) {
-             if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
+             if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

mm/swap_state.c struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, · line 887

Quoted 2 lines

mm/swap_state.c +0 -0

                      folio_put(folio);
                      return NULL;

I think that why not provide "mm" when it's available? For swapin_direct() called by do_swap_page(), mm can be provided. While, for swapin_direct() called by shmem swapin, mm will be NULL. We can even provide "mm" for __read_swap_cache_async() for VMA based swapin and for the fault address for cluster swapin.

Quoted 37 lines · Huang, Ying <[email protected]> 于2024年1月5日周五 15:16写道：

Quoted 34 lines · Kairui Song

From: Kairui Song <[email protected]>

Currently, mem_cgroup_swapin_charge_folio is always called with mm argument as NULL, except in swapin_direct.

swapin_direct is used when swapin should skip readahead and swapcache (SWP_SYNCHRONOUS_IO). Other caller paths of mem_cgroup_swapin_charge_folio are for swapin that should not skip readahead and cache.

This could cause swapin charging to behave differently depending on swap device. This currently didn't happen because the only call path of swapin_direct is the direct anon page fault path, where mm equals to current->mm, but will no longer be true if swapin_direct is shared and have other callers (eg, swapoff).

So make swapin_direct also passes NULL for mm, no feature change.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/swap_state.c +1 -1

index 6130de8d5226..d39c5369da21 100644
@@ -881,7 +881,7 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
      folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
                              vma, vmf->address, false);
      if (folio) {
-             if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
+             if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

Quoted 8 lines

mm/swap_state.c struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, · line 887

Quoted 2 lines

mm/swap_state.c +0 -0

                      folio_put(folio);
                      return NULL;

I think that why not provide "mm" when it's available? For swapin_direct() called by do_swap_page(), mm can be provided. While, for swapin_direct() called by shmem swapin, mm will be NULL. We can even provide "mm" for __read_swap_cache_async() for VMA based swapin and for the fault address for cluster swapin.

Hi, Ying.

Thanks for the comment.

Without modifying too much code, providing mm here will change swapin charge behaviour on swapoff, we discussed it previously:
https://lkml.org/lkml/2023/11/19/320

If we want to avoid the behavior change, we have to extend all direct and indirect callers of mem_cgroup_swapin_charge_folio to accept a standalone mm argument (including swapin_direct, swap_vma_readahead, swap_cluster_readahead, __read_swap_cache_async, swapin_entry_mpol, read_swap_cache_async, and some other path may need more audit), and sort things our case by case. I'm not sure if that's a good idea...

Simply dropping it here seems the easiest way to avoid such change.

Quoted 47 lines · Kairui Song

Quoted 37 lines · Huang, Ying <[email protected]> 于2024年1月5日周五 15:16写道：

Quoted 34 lines · Kairui Song

From: Kairui Song <[email protected]>

Currently, mem_cgroup_swapin_charge_folio is always called with mm argument as NULL, except in swapin_direct.

swapin_direct is used when swapin should skip readahead and swapcache (SWP_SYNCHRONOUS_IO). Other caller paths of mem_cgroup_swapin_charge_folio are for swapin that should not skip readahead and cache.

This could cause swapin charging to behave differently depending on swap device. This currently didn't happen because the only call path of swapin_direct is the direct anon page fault path, where mm equals to current->mm, but will no longer be true if swapin_direct is shared and have other callers (eg, swapoff).

So make swapin_direct also passes NULL for mm, no feature change.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/swap_state.c +1 -1

index 6130de8d5226..d39c5369da21 100644
@@ -881,7 +881,7 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
      folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
                              vma, vmf->address, false);
      if (folio) {
-             if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
+             if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

Quoted 8 lines

mm/swap_state.c struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, · line 887

Quoted 2 lines

mm/swap_state.c +0 -0

                      folio_put(folio);
                      return NULL;

I think that why not provide "mm" when it's available? For swapin_direct() called by do_swap_page(), mm can be provided. While, for swapin_direct() called by shmem swapin, mm will be NULL. We can even provide "mm" for __read_swap_cache_async() for VMA based swapin and for the fault address for cluster swapin.

Quoted 7 lines

Hi, Ying.

Thanks for the comment.

Without modifying too much code, providing mm here will change swapin charge behaviour on swapoff, we discussed it previously:
https://lkml.org/lkml/2023/11/19/320

It's better to use "lore" for kernel email link, for example,

https://lore.kernel.org/lkml/[email protected]/

This is more readable than the above link.

Quoted 8 lines

If we want to avoid the behavior change, we have to extend all direct and indirect callers of mem_cgroup_swapin_charge_folio to accept a standalone mm argument (including swapin_direct, swap_vma_readahead, swap_cluster_readahead, __read_swap_cache_async, swapin_entry_mpol, read_swap_cache_async, and some other path may need more audit), and sort things our case by case. I'm not sure if that's a good idea...

Quoted 1 line

Simply dropping it here seems the easiest way to avoid such change.

OK. Didn't realize that they are related directly. It appears that we always pass NULL as mm to mem_cgroup_swapin_charge_folio(). If so, please just remove the parameter.

--
Best Regards,
Huang, Ying

From: Kairui Song <[email protected]>

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

This commit makes swapoff make use of this new helper and now swapping off a 10G ZRAM (lzo-rle) is faster since readahead is skipped.

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 <[email protected]>

---
 mm/memory.c     | 21 +++++++--------------
 mm/swap.h       | 16 ++++------------
 mm/swap_state.c | 49 +++++++++++++++++++++++++++++++++----------------
 mm/swapfile.c   |  7 ++-----
 4 files changed, 46 insertions(+), 47 deletions(-)

mm/memory.c +7 -14

index 0165c8cad489..b56254a875f8 100644
@@ -3801,6 +3801,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	rmap_t rmap_flags = RMAP_NONE;
 	bool exclusive = false;
 	swp_entry_t entry;
+	bool swapcached;
 	pte_t pte;
 	vm_fault_t ret = 0;
 

@@ -3864,21 +3865,13 @@ 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;
+		folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
+				     vmf, &swapcached);
+		if (folio) {
+			page = folio_file_page(folio, swp_offset(entry));
+			if (swapcached)
+				swapcache = folio;
 		} else {
-			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
-						vmf);
-			if (page)
-				folio = page_folio(page);
-			swapcache = folio;
-		}
-
-		if (!folio) {
 			/*
 			 * Back out if somebody else faulted in this pte
 			 * while we released the pte lock.

mm/swap.h +4 -12

index 83eab7b67e77..502a2801f817 100644
@@ -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, bool *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, bool *swapcached)
 {
 	return NULL;
 }

mm/swap_state.c +0 -0

index d39c5369da21..66ff187aa5d3 100644

mm/swap_state.c +5 -4

 static inline bool swap_use_vma_readahead(void)
 {
 	return READ_ONCE(enable_vma_readahead) && !atomic_read(&nr_rotate_swap);

@@ -870,8 +875,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 +913,45 @@ 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 page from swap entry
  * @entry: swap entry of this memory
  * @gfp_mask: memory allocation flags
  * @vmf: fault information
+ * @swapcached: pointer to a bool used as indicator if the
+ *              page is swapped in through swapcache.
  *
  * Returns the struct page for entry and addr, after queueing swapin.
  *
- * It's a main entry function for swap readahead. By the configuration,

mm/swap_state.c +18 -9

 {
 	struct mempolicy *mpol;
-	pgoff_t ilx;
 	struct folio *folio;
+	pgoff_t ilx;
+	bool cached;
 
-	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 (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+		folio = swapin_direct(entry, gfp_mask, vmf);
+		cached = false;
+	} 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);
+		cached = true;
+	}
 
-	if (!folio)
-		return NULL;
-	return folio_file_page(folio, swp_offset(entry));
+	if (swapcached)
+		*swapcached = cached;
+
+	return folio;
 }
 
 #ifdef CONFIG_SYSFS

mm/swapfile.c +0 -1

index f7271504aa0a..ce4e6c10dce7 100644
@@ -1866,7 +1866,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,

@@ -1874,10 +1873,8 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 				.pmd = pmd,
 			};

Quoted 111 lines · Kairui Song

From: Kairui Song <[email protected]>

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

This commit makes swapoff make use of this new helper and now swapping off a 10G ZRAM (lzo-rle) is faster since readahead is skipped.

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 <[email protected]>

---
 mm/memory.c     | 21 +++++++--------------
 mm/swap.h       | 16 ++++------------
 mm/swap_state.c | 49 +++++++++++++++++++++++++++++++++----------------
 mm/swapfile.c   |  7 ++-----
 4 files changed, 46 insertions(+), 47 deletions(-)

mm/memory.c +7 -14

index 0165c8cad489..b56254a875f8 100644
@@ -3801,6 +3801,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
      rmap_t rmap_flags = RMAP_NONE;
      bool exclusive = false;
      swp_entry_t entry;
+     bool swapcached;
      pte_t pte;
      vm_fault_t ret = 0;
 

@@ -3864,21 +3865,13 @@ 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;
+             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
+                                  vmf, &swapcached);
+             if (folio) {
+                     page = folio_file_page(folio, swp_offset(entry));
+                     if (swapcached)
+                             swapcache = folio;
              } else {
-                     page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
-                                             vmf);
-                     if (page)
-                             folio = page_folio(page);
-                     swapcache = folio;
-             }
-
-             if (!folio) {
                      /*
                       * Back out if somebody else faulted in this pte
                       * while we released the pte lock.

mm/swap.h +4 -12

index 83eab7b67e77..502a2801f817 100644
@@ -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, bool *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, bool *swapcached)
 {
      return NULL;
 }

mm/swap_state.c +0 -0

index d39c5369da21..66ff187aa5d3 100644
@@ -316,6 +316,11 @@ void free_pages_and_swap_cache(struct encoded_page **pages, int nr)
      release_pages(pages, nr);
 }
 

mm/swap_state.c void free_pages_and_swap_cache(struct encoded_page **pages, int nr) · line 323

Quoted 5 lines

mm/swap_state.c +5 -0

+static inline bool swap_use_no_readahead(struct swap_info_struct *si, swp_entry_t entry)
+{
+     return data_race(si->flags & SWP_SYNCHRONOUS_IO) && __swap_count(entry) == 1;
+}
+

It appears that there's only one caller of the function in the same file? Why add a function?

Quoted 30 lines

mm/swap_state.c +5 -4

 static inline bool swap_use_vma_readahead(void)
 {
      return READ_ONCE(enable_vma_readahead) && !atomic_read(&nr_rotate_swap);

@@ -870,8 +875,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 +913,45 @@ 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 page from swap entry
  * @entry: swap entry of this memory
  * @gfp_mask: memory allocation flags
  * @vmf: fault information
+ * @swapcached: pointer to a bool used as indicator if the
+ *              page is swapped in through swapcache.
  *
  * Returns the struct page for entry and addr, after queueing swapin.
  *
- * It's a main entry function for swap readahead. By the configuration,

mm/swap_state.c struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, · line 931

Quoted 10 lines

mm/swap_state.c +5 -3

+ * It's a 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, bool *swapcached)

May be better to use

struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
struct vm_fault *vmf, struct folio **swapcache)

In this way, we can reduce the number of source lines in the caller.

From: Kairui Song <[email protected]>

Since all callers of swapin_entry need to check the swap cache first, we can merge this common routine into swapin_entry, so it can be shared and optimized later.

Also introduce a enum to better represent possible swap cache usage, and add some comments about it, make the usage of swap cache easier to understand.

Quoted 6 lines · Kairui Song

From: Kairui Song <[email protected]>

Since all callers of swapin_entry need to check the swap cache first, we can merge this common routine into swapin_entry, so it can be shared and optimized later.

Quoted 3 lines

Also introduce a enum to better represent possible swap cache usage, and add some comments about it, make the usage of swap cache easier to understand.

I don't find any benefit to do this. The code line number isn't reduced. The concept of swap cache isn't hided either.

--
Best Regards,
Huang, Ying

Quoted 9 lines · Huang, Ying <[email protected]> 于2024年1月8日周一 16:28写道：

Quoted 6 lines · Kairui Song

From: Kairui Song <[email protected]>

Since all callers of swapin_entry need to check the swap cache first, we can merge this common routine into swapin_entry, so it can be shared and optimized later.

Quoted 6 lines

Quoted 3 lines

Also introduce a enum to better represent possible swap cache usage, and add some comments about it, make the usage of swap cache easier to understand.

I don't find any benefit to do this. The code line number isn't reduced. The concept of swap cache isn't hided either.

Hi Ying

Thanks for the comments.

Maybe I should squash this with the following commit? The following commit want to do cache lookup in swapin_entry to avoid a redundant shadow lookup, it can help to improve the performance by about 4% for swapin path.

So it need to return a enum to represent cache status.

Further more, note the comments added here:

+/*
+ * Caller of swapin_entry may need to know the cache lookup result:
+ *
+ * SWAP_CACHE_HIT: cache hit, cached folio is retured.
+ * SWAP_CACHE_MISS: cache miss, folio is allocated, read from swap device
+ * and adde to swap cache, but still may return a cached
+ * folio if raced (check __read_swap_cache_async).
+ * SWAP_CACHE_BYPASS: cache miss, folio is new allocated and read
+ * from swap device bypassing the cache.
+ */

SWAP_CACHE_MISS might be inaccurate, this is not an issue introduced by this commit, but better exposed. May worth a fix later. So far I can see two benefits fixing it:
- More accurate maj/min page fault count.
- Note the PageHWPoison check in do_swap_page, it ignored the race case, if a page getting poisoned is raced with swapcache then it may not work as expected.

These are all minor issue indeed, some other optimization might also be doable, but at least a comment might be helpful.

Quoted 21 lines · Kairui Song

Quoted 9 lines · Huang, Ying <[email protected]> 于2024年1月8日周一 16:28写道：

Quoted 6 lines · Kairui Song

From: Kairui Song <[email protected]>

Since all callers of swapin_entry need to check the swap cache first, we can merge this common routine into swapin_entry, so it can be shared and optimized later.

Quoted 6 lines

Quoted 3 lines

Also introduce a enum to better represent possible swap cache usage, and add some comments about it, make the usage of swap cache easier to understand.

I don't find any benefit to do this. The code line number isn't reduced. The concept of swap cache isn't hided either.

Hi Ying

Thanks for the comments.

Quoted 4 lines

Maybe I should squash this with the following commit? The following commit want to do cache lookup in swapin_entry to avoid a redundant shadow lookup, it can help to improve the performance by about 4% for swapin path.

It's good to improve performance. But I don't think we must put swap_cache_get_folio() in swapin_entry() to do that. We can just get "shadow" from swap_cache_get_folio() and pass it to swapin_entry().

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
 		struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
 			    struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+				struct mempolicy *mpol, pgoff_t ilx,
+				enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
 	return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+		struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+	return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
 	return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-				  struct vm_fault *vmf, void *shadow)
+				   struct mempolicy *mpol, pgoff_t ilx,
+				   void *shadow)
 {
-	struct vm_area_struct *vma = vmf->vma;
 	struct folio *folio;
 
-	/* skip swapcache */
-	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-				vma, vmf->address, false);
+	folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+			mpol, ilx, numa_node_id());
 	if (folio) {
 		if (mem_cgroup_swapin_charge_folio(folio, NULL,
 						   GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
 		goto done;
 	}
 
+	mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
 	if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-		folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+		folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
 		cache_result = SWAP_CACHE_BYPASS;
 	} 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);
 		cache_result = SWAP_CACHE_MISS;
 	}
+	mpol_cond_put(mpol);
 done:
 	if (result)
 		*result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
 	return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+				struct mempolicy *mpol, pgoff_t ilx,
+				enum swap_cache_result *result)
+{
+	enum swap_cache_result cache_result;
+	void *shadow = NULL;
+	struct folio *folio;
+
+	folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+	if (folio) {
+		cache_result = SWAP_CACHE_HIT;
+	} else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+		folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+		cache_result = SWAP_CACHE_BYPASS;
+	} else {
+		folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+		cache_result = SWAP_CACHE_MISS;
+	}
+
+	if (result)
+		*result = cache_result;
+
+	return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
 				     struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 	do {
 		struct folio *folio;
 		unsigned long offset;
+		struct mempolicy *mpol;
 		unsigned char swp_count;
 		swp_entry_t entry;
+		pgoff_t ilx;
 		int ret;
 		pte_t ptent;
 
-		struct vm_fault vmf = {
-			.vma = vma,
-			.address = addr,
-			.real_address = addr,
-			.pmd = pmd,
-		};
-
 		if (!pte++) {
 			pte = pte_offset_map(pmd, addr);
 			if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 		pte_unmap(pte);
 		pte = NULL;

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

And I don't think it's a good abstraction to make swapin_entry_mpol() always use cluster swapin, while swapin_entry() will try to use vma swapin. I think we can add "struct mempolicy *mpol" and "pgoff_t ilx" to swapin_entry() as parameters, and use them if vmf == NULL. If we want to enforce cluster swapin in swapoff path, it will be better to add some comments to describe why.

Quoted 164 lines · Huang, Ying <[email protected]> 于2024年1月9日周二 09:11写道：

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

Quoted 14 lines

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

Hi Ying

Actually there is a swap_use_no_readahead check in swapin_entry_mpol, so when readahaed is not needed (SYNC_IO), it's just skipped.

And I think VMA readahead is not helpful swapoff, swapoff is already walking the VMA, mostly uninterrupted in kernel space. With VMA readahead or not, it will issue IO page by page.
The benchmark result I posted before is actually VMA readhead vs no-readahead for ZRAM, sorry I didn't make it clear. It's obvious no-readahead is faster.

For actual block device, cluster readahead might be a good choice for swapoff, since all pages will be read for swapoff, there has to be enough memory for all swapcached page to stay in memory or swapoff will fail anyway, and cluster read is faster for block devices.

Quoted 7 lines

And I don't think it's a good abstraction to make swapin_entry_mpol() always use cluster swapin, while swapin_entry() will try to use vma swapin. I think we can add "struct mempolicy *mpol" and "pgoff_t ilx" to swapin_entry() as parameters, and use them if vmf == NULL. If we want to enforce cluster swapin in swapoff path, it will be better to add some comments to describe why.

Good suggestion, I thought extending swapin_entry may make its arguments list get too long, but seems mpol and ilx are the only thing needed here. I'll update it.

Quoted 192 lines · Kairui Song

Quoted 164 lines · Huang, Ying <[email protected]> 于2024年1月9日周二 09:11写道：

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

Quoted 14 lines

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

Hi Ying

Actually there is a swap_use_no_readahead check in swapin_entry_mpol, so when readahaed is not needed (SYNC_IO), it's just skipped.

And I think VMA readahead is not helpful swapoff, swapoff is already walking the VMA, mostly uninterrupted in kernel space. With VMA readahead or not, it will issue IO page by page.
The benchmark result I posted before is actually VMA readhead vs no-readahead for ZRAM, sorry I didn't make it clear. It's obvious no-readahead is faster.

Quoted 4 lines

For actual block device, cluster readahead might be a good choice for swapoff, since all pages will be read for swapoff, there has to be enough memory for all swapcached page to stay in memory or swapoff will fail anyway, and cluster read is faster for block devices.

It's possible. But please run the tests on some actual block devices and show your results. Random memory accessing pattern should be tested, and the swap space usage should be > 50% to show some not so friendly situation.

Quoted 200 lines · Ying

Quoted 192 lines · Kairui Song

Quoted 164 lines · Huang, Ying <[email protected]> 于2024年1月9日周二 09:11写道：

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

Quoted 14 lines

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

Hi Ying

Actually there is a swap_use_no_readahead check in swapin_entry_mpol, so when readahaed is not needed (SYNC_IO), it's just skipped.

And I think VMA readahead is not helpful swapoff, swapoff is already walking the VMA, mostly uninterrupted in kernel space. With VMA readahead or not, it will issue IO page by page.
The benchmark result I posted before is actually VMA readhead vs no-readahead for ZRAM, sorry I didn't make it clear. It's obvious no-readahead is faster.

Quoted 4 lines

For actual block device, cluster readahead might be a good choice for swapoff, since all pages will be read for swapoff, there has to be enough memory for all swapcached page to stay in memory or swapoff will fail anyway, and cluster read is faster for block devices.

Quoted 5 lines

It's possible. But please run the tests on some actual block devices and show your results. Random memory accessing pattern should be tested, and the swap space usage should be > 50% to show some not so friendly situation.

Hi Ying,

I setup a test environment and did following test, and found that cluster readahaed for swapoff is actually much worse in default setup:

1. Setup MySQL server using 2G memcg, with 28G buffer pool, and 24G NVME swap
2. Stress test with sysbench for 15min.
3. Remove the 2G memcg limit and swapoff.

Before this patch, swapoff will take about 9m.
After this patch, swapoff will take about 30m.

After some analysis I found the reason is that cluster readahead is almost disabled (window == 1 or 2) during swapoff, because it will detect a very low hit rate on fragmented swap. But VMA readhead is much more aggressive here since swapoff is walking the VMA, with a very high hit rate.

But If I force cluster readahead to use a large window for swapoff, the swapoff performance boost by a lot:
By adding following change in swap_cluster_readahead:

if (unlikely(!(si->flags & SWP_WRITEOK)))
mask = max_t(unsigned long, 1 << READ_ONCE(page_cluster), PMD_SIZE
/ PAGE_SIZE) - 1;

The swapoff will take only 40s to finish, more than 10x faster than the VMA readahead path (9m), because VMA readhead is still doing 4K random IO just with a longer queue due to async readahead. But cluster readhead will be doing 2M IO now.
I think PMD size window is good here since it still keep a balance between good IO performance and the swapoff progress can still be interrupted, and the system is responsible. And in most cases we expect swapoff to success, if it fail, the RA windows should still keep the side effect of extra swapcache being generated acceptable.

But this showed a bad effect of ignoring mem policy. Actually this is not a new issue, cluster readhead is already violating VMA's mem policy since it does readhead only based on entry value not VMA, the entry being swapped in is not aware of which VMA it belongs.

And I was thinking, maybe we can just drop the mpol all the way, and use the nid from page shadow to alloc pages, that may save a lot of effort, and make cluster readhead more usable in general, also might simplify a lot of code. How do you think? If this is acceptable, I think I can send out a new series first and then rework this one later.

Quoted 212 lines · Kairui Song

Quoted 200 lines · Ying

Quoted 192 lines · Kairui Song

Quoted 164 lines · Huang, Ying <[email protected]> 于2024年1月9日周二 09:11写道：

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

Quoted 14 lines

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

Hi Ying

Actually there is a swap_use_no_readahead check in swapin_entry_mpol, so when readahaed is not needed (SYNC_IO), it's just skipped.

And I think VMA readahead is not helpful swapoff, swapoff is already walking the VMA, mostly uninterrupted in kernel space. With VMA readahead or not, it will issue IO page by page.
The benchmark result I posted before is actually VMA readhead vs no-readahead for ZRAM, sorry I didn't make it clear. It's obvious no-readahead is faster.

Quoted 4 lines

For actual block device, cluster readahead might be a good choice for swapoff, since all pages will be read for swapoff, there has to be enough memory for all swapcached page to stay in memory or swapoff will fail anyway, and cluster read is faster for block devices.

Quoted 5 lines

It's possible. But please run the tests on some actual block devices and show your results. Random memory accessing pattern should be tested, and the swap space usage should be > 50% to show some not so friendly situation.

Hi Ying,

I setup a test environment and did following test, and found that cluster readahaed for swapoff is actually much worse in default setup:

Quoted 6 lines

1. Setup MySQL server using 2G memcg, with 28G buffer pool, and 24G NVME swap
2. Stress test with sysbench for 15min.
3. Remove the 2G memcg limit and swapoff.

Before this patch, swapoff will take about 9m.
After this patch, swapoff will take about 30m.

Thanks for data!

Quoted 14 lines

After some analysis I found the reason is that cluster readahead is almost disabled (window == 1 or 2) during swapoff, because it will detect a very low hit rate on fragmented swap. But VMA readhead is much more aggressive here since swapoff is walking the VMA, with a very high hit rate.

But If I force cluster readahead to use a large window for swapoff, the swapoff performance boost by a lot:
By adding following change in swap_cluster_readahead:

if (unlikely(!(si->flags & SWP_WRITEOK)))
mask = max_t(unsigned long, 1 << READ_ONCE(page_cluster), PMD_SIZE
/ PAGE_SIZE) - 1;

Quoted 9 lines

The swapoff will take only 40s to finish, more than 10x faster than the VMA readahead path (9m), because VMA readhead is still doing 4K random IO just with a longer queue due to async readahead. But cluster readhead will be doing 2M IO now.
I think PMD size window is good here since it still keep a balance between good IO performance and the swapoff progress can still be interrupted, and the system is responsible. And in most cases we expect swapoff to success, if it fail, the RA windows should still keep the side effect of extra swapcache being generated acceptable.

swapoff performance isn't very important because swapoff is a very rare operation. It's OK to optimize it if the change is simple and doesn't compromise other stuff. But, as you said below, using large readahead window makes mempolicy issue more serious. Why isn't the original swapoff performance good enough for you?

Quoted 6 lines

But this showed a bad effect of ignoring mem policy. Actually this is not a new issue, cluster readhead is already violating VMA's mem policy since it does readhead only based on entry value not VMA, the entry being swapped in is not aware of which VMA it belongs.

Quoted 6 lines

And I was thinking, maybe we can just drop the mpol all the way, and use the nid from page shadow to alloc pages, that may save a lot of effort, and make cluster readhead more usable in general, also might simplify a lot of code. How do you think? If this is acceptable, I think I can send out a new series first and then rework this one later.

The "shadow" node can be reclaimed, please take a look at scan_shadow_nodes(). Although this hasn't been implemented, it may be implemented someday.

Quoted 248 lines · Ying

Quoted 212 lines · Kairui Song

Quoted 200 lines · Ying

Quoted 192 lines · Kairui Song

Quoted 164 lines · Huang, Ying <[email protected]> 于2024年1月9日周二 09:11写道：

Quoted 161 lines · Kairui Song

From: Kairui Song <[email protected]>

There are two places where swapin is not caused by direct anon page fault:
- shmem swapin, invoked indirectly through shmem mapping
- swapoff

They used to construct a pseudo vmfault struct for swapin function. Shmem has dropped the pseudo vmfault recently in commit ddc1a5cbc05d ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"). Swapoff path is still using one.

Introduce a helper for them both, this help save stack usage for swapoff path, and help apply a unified swapin cache and readahead policy check.

Due to missing vmfault info, the caller have to pass in mempolicy explicitly, make it different from swapin_entry and name it swapin_entry_mpol.

This commit convert swapoff to use this helper, follow-up commits will convert shmem to use it too.

Signed-off-by Kairui Song <[email protected]>

---
 mm/swap.h       |  9 +++++++++
 mm/swap_state.c | 40 ++++++++++++++++++++++++++++++++--------
 mm/swapfile.c   | 15 ++++++---------
 3 files changed, 47 insertions(+), 17 deletions(-)

mm/swap.h +9 -0

index 9180411afcfe..8f790a67b948 100644
@@ -73,6 +73,9 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
              struct mempolicy *mpol, pgoff_t ilx);
 struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
                          struct vm_fault *vmf, enum swap_cache_result *result);
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result);
 
 static inline unsigned int folio_swap_flags(struct folio *folio)
 {

@@ -109,6 +112,12 @@ static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
      return NULL;
 }
 
+static inline struct page *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+             struct mempolicy *mpol, pgoff_t ilx, enum swap_cache_result *result)
+{
+     return NULL;
+}
+
 static inline int swap_writepage(struct page *p, struct writeback_control *wbc)
 {
      return 0;

mm/swap_state.c +32 -8

index 21badd4f0fc7..3edf4b63158d 100644
@@ -880,14 +880,13 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
  * in.
  */
 static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
-                               struct vm_fault *vmf, void *shadow)
+                                struct mempolicy *mpol, pgoff_t ilx,
+                                void *shadow)
 {
-     struct vm_area_struct *vma = vmf->vma;
      struct folio *folio;
 
-     /* skip swapcache */
-     folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0,
-                             vma, vmf->address, false);
+     folio = (struct folio *)alloc_pages_mpol(gfp_mask, 0,
+                     mpol, ilx, numa_node_id());
      if (folio) {
              if (mem_cgroup_swapin_charge_folio(folio, NULL,
                                                 GFP_KERNEL, entry)) {

@@ -943,18 +942,18 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
              goto done;
      }
 
+     mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
      if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
-             folio = swapin_direct(entry, gfp_mask, vmf, shadow);
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
              cache_result = SWAP_CACHE_BYPASS;
      } 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);
              cache_result = SWAP_CACHE_MISS;
      }
+     mpol_cond_put(mpol);
 done:
      if (result)
              *result = cache_result;

@@ -962,6 +961,31 @@ struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
      return folio;
 }
 
+struct folio *swapin_entry_mpol(swp_entry_t entry, gfp_t gfp_mask,
+                             struct mempolicy *mpol, pgoff_t ilx,
+                             enum swap_cache_result *result)
+{
+     enum swap_cache_result cache_result;
+     void *shadow = NULL;
+     struct folio *folio;
+
+     folio = swap_cache_get_folio(entry, NULL, 0, &shadow);
+     if (folio) {
+             cache_result = SWAP_CACHE_HIT;
+     } else if (swap_use_no_readahead(swp_swap_info(entry), entry)) {
+             folio = swapin_direct(entry, gfp_mask, mpol, ilx, shadow);
+             cache_result = SWAP_CACHE_BYPASS;
+     } else {
+             folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
+             cache_result = SWAP_CACHE_MISS;
+     }
+
+     if (result)
+             *result = cache_result;
+
+     return folio;
+}
+
 #ifdef CONFIG_SYSFS
 static ssize_t vma_ra_enabled_show(struct kobject *kobj,
                                   struct kobj_attribute *attr, char *buf)

mm/swapfile.c +2 -7

index 5aa44de11edc..2f77bf143af8 100644
@@ -1840,18 +1840,13 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
      do {
              struct folio *folio;
              unsigned long offset;
+             struct mempolicy *mpol;
              unsigned char swp_count;
              swp_entry_t entry;
+             pgoff_t ilx;
              int ret;
              pte_t ptent;
 
-             struct vm_fault vmf = {
-                     .vma = vma,
-                     .address = addr,
-                     .real_address = addr,
-                     .pmd = pmd,
-             };
-
              if (!pte++) {
                      pte = pte_offset_map(pmd, addr);
                      if (!pte)

@@ -1871,8 +1866,10 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
              pte_unmap(pte);
              pte = NULL;
 

Quoted 14 lines

mm/swapfile.c static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd, · line 1875

Quoted 9 lines

mm/swapfile.c +4 -2

-             folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
-                                  &vmf, NULL);
+             mpol = get_vma_policy(vma, addr, 0, &ilx);
+             folio = swapin_entry_mpol(entry, GFP_HIGHUSER_MOVABLE,
+                                       mpol, ilx, NULL);
+             mpol_cond_put(mpol);
              if (!folio) {
                      /*
                       * The entry could have been freed, and will not

IIUC, after the change, we will always use cluster readahead for swapoff. This may be OK. But, at least we need some test results which show that this will not cause any issue for this behavior change. And the behavior change should be described explicitly in patch description.

Hi Ying

Actually there is a swap_use_no_readahead check in swapin_entry_mpol, so when readahaed is not needed (SYNC_IO), it's just skipped.

And I think VMA readahead is not helpful swapoff, swapoff is already walking the VMA, mostly uninterrupted in kernel space. With VMA readahead or not, it will issue IO page by page.
The benchmark result I posted before is actually VMA readhead vs no-readahead for ZRAM, sorry I didn't make it clear. It's obvious no-readahead is faster.

Quoted 4 lines

For actual block device, cluster readahead might be a good choice for swapoff, since all pages will be read for swapoff, there has to be enough memory for all swapcached page to stay in memory or swapoff will fail anyway, and cluster read is faster for block devices.

Quoted 5 lines

It's possible. But please run the tests on some actual block devices and show your results. Random memory accessing pattern should be tested, and the swap space usage should be > 50% to show some not so friendly situation.

Hi Ying,

I setup a test environment and did following test, and found that cluster readahaed for swapoff is actually much worse in default setup:

Quoted 6 lines

1. Setup MySQL server using 2G memcg, with 28G buffer pool, and 24G NVME swap
2. Stress test with sysbench for 15min.
3. Remove the 2G memcg limit and swapoff.

Before this patch, swapoff will take about 9m.
After this patch, swapoff will take about 30m.

Thanks for data!

Quoted 14 lines

After some analysis I found the reason is that cluster readahead is almost disabled (window == 1 or 2) during swapoff, because it will detect a very low hit rate on fragmented swap. But VMA readhead is much more aggressive here since swapoff is walking the VMA, with a very high hit rate.

But If I force cluster readahead to use a large window for swapoff, the swapoff performance boost by a lot:
By adding following change in swap_cluster_readahead:

if (unlikely(!(si->flags & SWP_WRITEOK)))
mask = max_t(unsigned long, 1 << READ_ONCE(page_cluster), PMD_SIZE
/ PAGE_SIZE) - 1;

Quoted 9 lines

The swapoff will take only 40s to finish, more than 10x faster than the VMA readahead path (9m), because VMA readhead is still doing 4K random IO just with a longer queue due to async readahead. But cluster readhead will be doing 2M IO now.
I think PMD size window is good here since it still keep a balance between good IO performance and the swapoff progress can still be interrupted, and the system is responsible. And in most cases we expect swapoff to success, if it fail, the RA windows should still keep the side effect of extra swapcache being generated acceptable.

Quoted 5 lines

swapoff performance isn't very important because swapoff is a very rare operation. It's OK to optimize it if the change is simple and doesn't compromise other stuff. But, as you said below, using large readahead window makes mempolicy issue more serious. Why isn't the original swapoff performance good enough for you?

Thanks for the reply.

I think I'll just keep the original VMA readahead policy here then. Just I noticed that VMA readhead will also violate ranged memory policy too... That's some different issue, looks trivial though.

Quoted 13 lines

Quoted 6 lines

But this showed a bad effect of ignoring mem policy. Actually this is not a new issue, cluster readhead is already violating VMA's mem policy since it does readhead only based on entry value not VMA, the entry being swapped in is not aware of which VMA it belongs.

Quoted 6 lines

And I was thinking, maybe we can just drop the mpol all the way, and use the nid from page shadow to alloc pages, that may save a lot of effort, and make cluster readhead more usable in general, also might simplify a lot of code. How do you think? If this is acceptable, I think I can send out a new series first and then rework this one later.

Quoted 3 lines

The "shadow" node can be reclaimed, please take a look at scan_shadow_nodes(). Although this hasn't been implemented, it may be implemented someday.

Right, I noticed upstream commit 5649d113ffce ("swap_state: update shadow_nodes for anonymous page") started reclaiming anon pages shadows now, thanks for the info.