Skip to contentSkip to conversation

swap readahead clean up

PATCH RESEND In review [email protected] 2 participants 8 messages Feb 20, 2018 – Feb 26, 2018

Subsystems (4)

v1 of 2 In review Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover swap readahead clean up

From: Minchan Kim <[email protected]>

This patchset cleans up recent added vma-based readahead code via unifying cluster-based readahead.

Resent based on mmotm-2018-02-06-16-41.

Minchan Kim (2):
mm: swap: clean up swap readahead
mm: swap: unify cluster-based and vma-based swap readahead

 include/linux/swap.h |  38 +++---------
 mm/memory.c          |  33 +++--------
 mm/shmem.c           |   5 +-
 mm/swap_state.c      | 137 +++++++++++++++++++++++++++----------------
 4 files changed, 105 insertions(+), 108 deletions(-)
Signature · 7 lines
2.16.1.291.g4437f3f132-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw

PATCH RESEND 1/2 mm: swap: clean up swap readahead

From: Minchan Kim <[email protected]>

When I see recent change of swap readahead, I am very unhappy about current code structure which diverges two swap readahead algorithm in do_swap_page. This patch is to clean it up.

Main motivation is that fault handler doesn't need to be aware of readahead algorithms but just should call swapin_readahead.

As first step, this patch cleans up a little bit but not perfect (I just separate for review easier) so next patch will make the goal complete.

---
 include/linux/swap.h | 17 ++-------
 mm/memory.c          | 26 +++----------
 mm/swap_state.c      | 89 ++++++++++++++++++++++++--------------------
 3 files changed, 57 insertions(+), 75 deletions(-)
include/linux/swap.h +3 -14
index a1a3f4ed94ce..fa92177d863e 100644
@@ -424,12 +424,8 @@ extern struct page *__read_swap_cache_async(swp_entry_t, gfp_t,
 			bool *new_page_allocated);
 extern struct page *swapin_readahead(swp_entry_t, gfp_t,
 			struct vm_area_struct *vma, unsigned long addr);
-
-extern struct page *swap_readahead_detect(struct vm_fault *vmf,
-					  struct vma_swap_readahead *swap_ra);
 extern struct page *do_swap_page_readahead(swp_entry_t fentry, gfp_t gfp_mask,
-					   struct vm_fault *vmf,
-					   struct vma_swap_readahead *swap_ra);
+					   struct vm_fault *vmf);
 
 /* linux/mm/swapfile.c */
 extern atomic_long_t nr_swap_pages;
@@ -548,15 +544,8 @@ static inline bool swap_use_vma_readahead(void)
 	return false;
 }
 
-static inline struct page *swap_readahead_detect(
-	struct vm_fault *vmf, struct vma_swap_readahead *swap_ra)
-{
-	return NULL;
-}
-
-static inline struct page *do_swap_page_readahead(
-	swp_entry_t fentry, gfp_t gfp_mask,
-	struct vm_fault *vmf, struct vma_swap_readahead *swap_ra)
+static inline struct page *do_swap_page_readahead(swp_entry_t fentry,
+				gfp_t gfp_mask, struct vm_fault *vmf)
 {
 	return NULL;
 }
mm/memory.c +6 -20
index 5ec6433d6a5c..5b6e29d927c8 100644
@@ -2883,26 +2883,16 @@ EXPORT_SYMBOL(unmap_mapping_range);
 int do_swap_page(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
-	struct page *page = NULL, *swapcache = NULL;
+	struct page *page = NULL, *swapcache;
 	struct mem_cgroup *memcg;
-	struct vma_swap_readahead swap_ra;
 	swp_entry_t entry;
 	pte_t pte;
 	int locked;
 	int exclusive = 0;
 	int ret = 0;
-	bool vma_readahead = swap_use_vma_readahead();
 
-	if (vma_readahead) {
-		page = swap_readahead_detect(vmf, &swap_ra);
-		swapcache = page;
-	}
-
-	if (!pte_unmap_same(vma->vm_mm, vmf->pmd, vmf->pte, vmf->orig_pte)) {
-		if (page)
-			put_page(page);
+	if (!pte_unmap_same(vma->vm_mm, vmf->pmd, vmf->pte, vmf->orig_pte))
 		goto out;
-	}
 
 	entry = pte_to_swp_entry(vmf->orig_pte);
 	if (unlikely(non_swap_entry(entry))) {
@@ -2928,11 +2918,8 @@ int do_swap_page(struct vm_fault *vmf)
 
 
 	delayacct_set_flag(DELAYACCT_PF_SWAPIN);
-	if (!page) {
-		page = lookup_swap_cache(entry, vma_readahead ? vma : NULL,
-					 vmf->address);
-		swapcache = page;
-	}
+	page = lookup_swap_cache(entry, vma, vmf->address);
+	swapcache = page;
 
 	if (!page) {
 		struct swap_info_struct *si = swp_swap_info(entry);
@@ -2949,9 +2936,9 @@ int do_swap_page(struct vm_fault *vmf)
 				swap_readpage(page, true);
 			}
 		} else {
-			if (vma_readahead)
+			if (swap_use_vma_readahead())
 				page = do_swap_page_readahead(entry,
-					GFP_HIGHUSER_MOVABLE, vmf, &swap_ra);
+					GFP_HIGHUSER_MOVABLE, vmf);
 			else
 				page = swapin_readahead(entry,
 				       GFP_HIGHUSER_MOVABLE, vma, vmf->address);
@@ -2982,7 +2969,6 @@ int do_swap_page(struct vm_fault *vmf)
 		 */
 		ret = VM_FAULT_HWPOISON;
 		delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
-		swapcache = page;
 		goto out_release;
 	}
 
mm/swap_state.c +48 -41

175 lines not on this page · raw message

Signature · 7 lines
2.16.1.291.g4437f3f132-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

Re: [PATCH RESEND 1/2] mm: swap: clean up swap readahead

<[email protected]> writes:
[snip]

Quoted 8 lines
mm/swap_state.c +0 -0
index 39ae7cfad90f..c56cce64b2c3 100644
@@ -332,32 +332,38 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 342

Quoted 10 lines
mm/swap_state.c +3 -2
-	unsigned long ra_info;
-	int win, hits, readahead;
 
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
+		bool vma_ra = swap_use_vma_readahead();
+		bool readahead = TestClearPageReadahead(page);
+

TestClearPageReadahead() cannot be called for compound page. As in

PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 345

Quoted 4 lines
mm/swap_state.c +0 -1
 		INC_CACHE_INFO(find_success);
 		if (unlikely(PageTransCompound(page)))
 			return page;
-		readahead = TestClearPageReadahead(page);

So we can only call it here after checking whether page is compound.

Best Regards,
Huang, Ying

Quoted 19 lines
mm/swap_state.c +9 -4
-		if (vma) {
-			ra_info = GET_SWAP_RA_VAL(vma);
-			win = SWAP_RA_WIN(ra_info);
-			hits = SWAP_RA_HITS(ra_info);
+
+		if (vma && vma_ra) {
+			unsigned long ra_val;
+			int win, hits;
+
+			ra_val = GET_SWAP_RA_VAL(vma);
+			win = SWAP_RA_WIN(ra_val);
+			hits = SWAP_RA_HITS(ra_val);
 			if (readahead)
 				hits = min_t(int, hits + 1, SWAP_RA_HITS_MAX);
 			atomic_long_set(&vma->swap_readahead_info,
 					SWAP_RA_VAL(addr, win, hits));
 		}
+
 		if (readahead) {

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 369

Quoted 10 lines
mm/swap_state.c +2 -1
 			count_vm_event(SWAP_RA_HIT);
-			if (!vma)
+			if (!vma || !vma_ra)
 				atomic_inc(&swapin_readahead_hits);
 		}
 	}
+
 	return page;
 }
 

[snip]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#3 lore raw

Re: [PATCH RESEND 1/2] mm: swap: clean up swap readahead

Quoted 27 lines · Ying

<[email protected]> writes:
[snip]

Quoted 8 lines
mm/swap_state.c +0 -0
index 39ae7cfad90f..c56cce64b2c3 100644
@@ -332,32 +332,38 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 342

Quoted 10 lines
mm/swap_state.c +3 -2
-	unsigned long ra_info;
-	int win, hits, readahead;
 
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
+		bool vma_ra = swap_use_vma_readahead();
+		bool readahead = TestClearPageReadahead(page);
+

TestClearPageReadahead() cannot be called for compound page. As in

PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

Quoted 6 lines

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 345

Quoted 4 lines
mm/swap_state.c +0 -1
 		INC_CACHE_INFO(find_success);
 		if (unlikely(PageTransCompound(page)))
 			return page;
-		readahead = TestClearPageReadahead(page);

So we can only call it here after checking whether page is compound.

Hi Huang,

Thanks for cathing this.
However, I don't see the reason we should rule out THP page for readahead marker. Could't we relax the rule?

I hope we can do so that we could remove PageTransCompound check for readahead marker, which makes code ugly.

#4 lore raw

Re: [PATCH RESEND 1/2] mm: swap: clean up swap readahead

Quoted 62 lines · Minchan Kim
Quoted 27 lines · Ying

<[email protected]> writes:
[snip]

Quoted 8 lines
mm/swap_state.c +0 -0
index 39ae7cfad90f..c56cce64b2c3 100644
@@ -332,32 +332,38 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 342

Quoted 10 lines
mm/swap_state.c +3 -2
-	unsigned long ra_info;
-	int win, hits, readahead;
 
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
+		bool vma_ra = swap_use_vma_readahead();
+		bool readahead = TestClearPageReadahead(page);
+

TestClearPageReadahead() cannot be called for compound page. As in

PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

Quoted 6 lines

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 345

Quoted 4 lines
mm/swap_state.c +0 -1
 		INC_CACHE_INFO(find_success);
 		if (unlikely(PageTransCompound(page)))
 			return page;
-		readahead = TestClearPageReadahead(page);

So we can only call it here after checking whether page is compound.

Hi Huang,

Thanks for cathing this.
However, I don't see the reason we should rule out THP page for readahead marker. Could't we relax the rule?

I hope we can do so that we could remove PageTransCompound check for readahead marker, which makes code ugly.

From 748b084d5c3960ec2418d8c51a678aada30f1072 Mon Sep 17 00:00:00 2001
From: Minchan Kim <[email protected]>
Date: Mon, 26 Feb 2018 13:46:43 +0900
Subject: [PATCH] mm: relax policy for PG_readahead

This flag is in use for anon THP page so let's relax it.

Cc Kirill A. Shutemov <[email protected]>
---
 include/linux/page-flags.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/page-flags.h +0 -0
index e34a27727b9a..f12d4dfae580 100644

include/linux/page-flags.h PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL) · line 325

Quoted 10 lines
include/linux/page-flags.h +2 -2
@@ -318,8 +318,8 @@ PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL)
 /* PG_readahead is only used for reads; PG_reclaim is only for writes */
 PAGEFLAG(Reclaim, reclaim, PF_NO_TAIL)
 	TESTCLEARFLAG(Reclaim, reclaim, PF_NO_TAIL)
-PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
-	TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)
+PAGEFLAG(Readahead, reclaim, PF_NO_TAIL)
+	TESTCLEARFLAG(Readahead, reclaim, PF_NO_TAIL)
 
 #ifdef CONFIG_HIGHMEM
 /*

We never set Readahead bit for THP in reality. The original code acts as document for this. I don't think it is a good idea to change this without a good reason.

Best Regards,
Huang, Ying

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw

Re: [PATCH RESEND 1/2] mm: swap: clean up swap readahead

Quoted 72 lines · Ying
Quoted 70 lines · Minchan Kim
Quoted 27 lines · Ying

<[email protected]> writes:
[snip]

Quoted 8 lines
mm/swap_state.c +0 -0
index 39ae7cfad90f..c56cce64b2c3 100644
@@ -332,32 +332,38 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 342

Quoted 10 lines
mm/swap_state.c +3 -2
-	unsigned long ra_info;
-	int win, hits, readahead;
 
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
+		bool vma_ra = swap_use_vma_readahead();
+		bool readahead = TestClearPageReadahead(page);
+

TestClearPageReadahead() cannot be called for compound page. As in

PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

Quoted 6 lines

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 345

Quoted 4 lines
mm/swap_state.c +0 -1
 		INC_CACHE_INFO(find_success);
 		if (unlikely(PageTransCompound(page)))
 			return page;
-		readahead = TestClearPageReadahead(page);

So we can only call it here after checking whether page is compound.

Hi Huang,

Thanks for cathing this.
However, I don't see the reason we should rule out THP page for readahead marker. Could't we relax the rule?

I hope we can do so that we could remove PageTransCompound check for readahead marker, which makes code ugly.

From 748b084d5c3960ec2418d8c51a678aada30f1072 Mon Sep 17 00:00:00 2001
From: Minchan Kim <[email protected]>
Date: Mon, 26 Feb 2018 13:46:43 +0900
Subject: [PATCH] mm: relax policy for PG_readahead

This flag is in use for anon THP page so let's relax it.

Cc Kirill A. Shutemov <[email protected]>
---
 include/linux/page-flags.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/page-flags.h +2 -2
index e34a27727b9a..f12d4dfae580 100644
@@ -318,8 +318,8 @@ PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL)
 /* PG_readahead is only used for reads; PG_reclaim is only for writes */
 PAGEFLAG(Reclaim, reclaim, PF_NO_TAIL)
 	TESTCLEARFLAG(Reclaim, reclaim, PF_NO_TAIL)
-PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
-	TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)
+PAGEFLAG(Readahead, reclaim, PF_NO_TAIL)
+	TESTCLEARFLAG(Readahead, reclaim, PF_NO_TAIL)
 
Quoted 6 lines

include/linux/page-flags.h PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL) · line 325

Quoted 2 lines
include/linux/page-flags.h +0 -0
 #ifdef CONFIG_HIGHMEM
 /*

We never set Readahead bit for THP in reality. The original code acts as document for this. I don't think it is a good idea to change this without a good reason.

I don't like such divergence so that we don't need to care about whether the page is THP or not. However, there is pointless to confuse ra stat counters, too. How about this?

mm/swap_state.c +8 -5
index 8dde719e973c..e169d137d27c 100644
@@ -348,12 +348,17 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 	INC_CACHE_INFO(find_total);
 	if (page) {
 		bool vma_ra = swap_use_vma_readahead();
-		bool readahead = TestClearPageReadahead(page);
+		bool readahead;
 
 		INC_CACHE_INFO(find_success);
+		/*
+		 * At the moment, we doesn't support PG_readahead for anon THP
+		 * so let's bail out rather than confusing the readahead stat.
+		 */
 		if (unlikely(PageTransCompound(page)))
 			return page;
 
+		readahead = TestClearPageReadahead(page);
 		if (vma && vma_ra) {
 			unsigned long ra_val;
 			int win, hits;
@@ -608,8 +613,7 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
 			continue;
 		if (page_allocated) {
 			swap_readpage(page, false);
-			if (offset != entry_offset &&
-			    likely(!PageTransCompound(page))) {
+			if (offset != entry_offset) {
 				SetPageReadahead(page);
 				count_vm_event(SWAP_RA);
 			}
@@ -772,8 +776,7 @@ struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
 			continue;
 		if (page_allocated) {
 			swap_readpage(page, false);
-			if (i != ra_info.offset &&
-			    likely(!PageTransCompound(page))) {
+			if (i != ra_info.offset) {
 				SetPageReadahead(page);
 				count_vm_event(SWAP_RA);
 			}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw

Re: [PATCH RESEND 1/2] mm: swap: clean up swap readahead

Quoted 118 lines · Minchan Kim
Quoted 72 lines · Ying
Quoted 70 lines · Minchan Kim
Quoted 27 lines · Ying

<[email protected]> writes:
[snip]

Quoted 8 lines
mm/swap_state.c +0 -0
index 39ae7cfad90f..c56cce64b2c3 100644
@@ -332,32 +332,38 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 			       unsigned long addr)
 {
 	struct page *page;

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma, · line 342

Quoted 10 lines
mm/swap_state.c +3 -2
-	unsigned long ra_info;
-	int win, hits, readahead;
 
 	page = find_get_page(swap_address_space(entry), swp_offset(entry));
 
 	INC_CACHE_INFO(find_total);
 	if (page) {
+		bool vma_ra = swap_use_vma_readahead();
+		bool readahead = TestClearPageReadahead(page);
+

TestClearPageReadahead() cannot be called for compound page. As in

PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

Quoted 6 lines

mm/swap_state.c struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,

Quoted 4 lines
mm/swap_state.c +0 -1
 		INC_CACHE_INFO(find_success);
 		if (unlikely(PageTransCompound(page)))
 			return page;
-		readahead = TestClearPageReadahead(page);

So we can only call it here after checking whether page is compound.

Hi Huang,

Thanks for cathing this.
However, I don't see the reason we should rule out THP page for readahead marker. Could't we relax the rule?

I hope we can do so that we could remove PageTransCompound check for readahead marker, which makes code ugly.

From 748b084d5c3960ec2418d8c51a678aada30f1072 Mon Sep 17 00:00:00 2001
From: Minchan Kim <[email protected]>
Date: Mon, 26 Feb 2018 13:46:43 +0900
Subject: [PATCH] mm: relax policy for PG_readahead

This flag is in use for anon THP page so let's relax it.

Cc Kirill A. Shutemov <[email protected]>
---
 include/linux/page-flags.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/page-flags.h +2 -2
index e34a27727b9a..f12d4dfae580 100644
@@ -318,8 +318,8 @@ PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL)
 /* PG_readahead is only used for reads; PG_reclaim is only for writes */
 PAGEFLAG(Reclaim, reclaim, PF_NO_TAIL)
 	TESTCLEARFLAG(Reclaim, reclaim, PF_NO_TAIL)
-PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
-	TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)
+PAGEFLAG(Readahead, reclaim, PF_NO_TAIL)
+	TESTCLEARFLAG(Readahead, reclaim, PF_NO_TAIL)
 
Quoted 6 lines

include/linux/page-flags.h PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_TAIL) · line 325

Quoted 2 lines
include/linux/page-flags.h +0 -0
 #ifdef CONFIG_HIGHMEM
 /*

We never set Readahead bit for THP in reality. The original code acts as document for this. I don't think it is a good idea to change this without a good reason.

I don't like such divergence so that we don't need to care about whether the page is THP or not. However, there is pointless to confuse ra stat counters, too. How about this?

mm/swap_state.c +7 -3
index 8dde719e973c..e169d137d27c 100644
@@ -348,12 +348,17 @@ struct page *lookup_swap_cache(swp_entry_t entry, struct vm_area_struct *vma,
 	INC_CACHE_INFO(find_total);
 	if (page) {
 		bool vma_ra = swap_use_vma_readahead();
-		bool readahead = TestClearPageReadahead(page);
+		bool readahead;
 
 		INC_CACHE_INFO(find_success);
+		/*
+		 * At the moment, we doesn't support PG_readahead for anon THP
+		 * so let's bail out rather than confusing the readahead stat.
+		 */
 		if (unlikely(PageTransCompound(page)))
 			return page;
 
+		readahead = TestClearPageReadahead(page);
 		if (vma && vma_ra) {
 			unsigned long ra_val;
 			int win, hits;
@@ -608,8 +613,7 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
 			continue;
 		if (page_allocated) {
 			swap_readpage(page, false);
-			if (offset != entry_offset &&
-			    likely(!PageTransCompound(page))) {
+			if (offset != entry_offset) {
 				SetPageReadahead(page);
 				count_vm_event(SWAP_RA);
 			}

mm/swap_state.c struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask, · line 782

Quoted 9 lines
mm/swap_state.c +1 -2
@@ -772,8 +776,7 @@ struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
 			continue;
 		if (page_allocated) {
 			swap_readpage(page, false);
-			if (i != ra_info.offset &&
-			    likely(!PageTransCompound(page))) {
+			if (i != ra_info.offset) {
 				SetPageReadahead(page);
 				count_vm_event(SWAP_RA);
 			}

This looks good for me. Thanks!

Best Regards,
Huang, Ying

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw

PATCH RESEND 2/2 mm: swap: unify cluster-based and vma-based swap readahead

In reply to [email protected]

From: Minchan Kim <[email protected]>

This patch makes do_swap_page() not need to be aware of two different swap readahead algorithms. Just unify cluster-based and vma-based readahead function call.

---
 include/linux/swap.h | 27 ++++++++-----------------
 mm/memory.c          | 11 ++++------
 mm/shmem.c           |  5 ++++-
 mm/swap_state.c      | 48 ++++++++++++++++++++++++++++++++++----------
 4 files changed, 53 insertions(+), 38 deletions(-)
include/linux/swap.h +8 -19
index fa92177d863e..2417d288e016 100644
@@ -400,7 +400,6 @@ int generic_swapfile_activate(struct swap_info_struct *, struct file *,
 #define SWAP_ADDRESS_SPACE_SHIFT	14
 #define SWAP_ADDRESS_SPACE_PAGES	(1 << SWAP_ADDRESS_SPACE_SHIFT)
 extern struct address_space *swapper_spaces[];
-extern bool swap_vma_readahead;
 #define swap_address_space(entry)			    \
 	(&swapper_spaces[swp_type(entry)][swp_offset(entry) \
 		>> SWAP_ADDRESS_SPACE_SHIFT])
@@ -422,10 +421,10 @@ extern struct page *read_swap_cache_async(swp_entry_t, gfp_t,
 extern struct page *__read_swap_cache_async(swp_entry_t, gfp_t,
 			struct vm_area_struct *vma, unsigned long addr,
 			bool *new_page_allocated);
-extern struct page *swapin_readahead(swp_entry_t, gfp_t,
-			struct vm_area_struct *vma, unsigned long addr);
-extern struct page *do_swap_page_readahead(swp_entry_t fentry, gfp_t gfp_mask,
-					   struct vm_fault *vmf);
+extern struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
+				struct vm_fault *vmf);
+extern struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
+				struct vm_fault *vmf);
 
 /* linux/mm/swapfile.c */
 extern atomic_long_t nr_swap_pages;
@@ -433,11 +432,6 @@ extern long total_swap_pages;
 extern atomic_t nr_rotate_swap;
 extern bool has_usable_swap(void);
 
-static inline bool swap_use_vma_readahead(void)
-{
-	return READ_ONCE(swap_vma_readahead) && !atomic_read(&nr_rotate_swap);
-}
-
 /* Swap 50% full? Release swapcache more aggressively.. */
 static inline bool vm_swap_full(void)
 {
@@ -533,19 +527,14 @@ static inline void put_swap_page(struct page *page, swp_entry_t swp)
 {
 }
 
-static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
-			struct vm_area_struct *vma, unsigned long addr)
+static inline struct page *swap_cluster_readahead(swp_entry_t entry,
+				gfp_t gfp_mask, struct vm_fault *vmf)
 {
 	return NULL;
 }
 
-static inline bool swap_use_vma_readahead(void)
-{
-	return false;
-}
-
-static inline struct page *do_swap_page_readahead(swp_entry_t fentry,
-				gfp_t gfp_mask, struct vm_fault *vmf)
+static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
+			struct vm_fault *vmf)
 {
 	return NULL;
 }
mm/memory.c +4 -7
index 5b6e29d927c8..394078c97a98 100644
@@ -2927,7 +2927,8 @@ int do_swap_page(struct vm_fault *vmf)
 		if (si->flags & SWP_SYNCHRONOUS_IO &&
 				__swap_count(si, entry) == 1) {
 			/* skip swapcache */
-			page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vmf->address);
+			page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma,
+							vmf->address);
 			if (page) {
 				__SetPageLocked(page);
 				__SetPageSwapBacked(page);
@@ -2936,12 +2937,8 @@ int do_swap_page(struct vm_fault *vmf)
 				swap_readpage(page, true);
 			}
 		} else {
-			if (swap_use_vma_readahead())
-				page = do_swap_page_readahead(entry,
-					GFP_HIGHUSER_MOVABLE, vmf);
-			else
-				page = swapin_readahead(entry,
-				       GFP_HIGHUSER_MOVABLE, vma, vmf->address);
+			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
+						vmf);
 			swapcache = page;
 		}
 
mm/shmem.c +4 -1
index 1907688b75ee..e493c5095b5f 100644
@@ -1413,9 +1413,12 @@ static struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
 {
 	struct vm_area_struct pvma;
 	struct page *page;
+	struct vm_fault vmf;
 
 	shmem_pseudo_vma_init(&pvma, info, index);
-	page = swapin_readahead(swap, gfp, &pvma, 0);
+	vmf.vma = &pvma;
+	vmf.address = 0;
+	page = swap_cluster_readahead(swap, gfp, &vmf);
 	shmem_pseudo_vma_destroy(&pvma);
 
 	return page;
mm/swap_state.c +37 -11

104 lines not on this page · raw message

Signature · 7 lines
2.16.1.291.g4437f3f132-goog

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>