mm,vmscan: only evict file pages when we have plenty

RFC PATCH In review Rik van Riel 3 participants 5 messages Oct 30, 2012 – Oct 31, 2012

Subsystems (3)

Memory management MGLRU MM reclaim

lore

Share & tools

lore

Rik van Riel

author

2012-10-30 18:39 UTC #0 lore raw

RFC PATCH 1/1 mm,vmscan: only evict file pages when we have plenty

If we have more inactive file pages than active file pages, we skip scanning the active file pages alltogether, with the idea that we do not want to evict the working set when there is plenty of streaming IO in the cache.

However, the code forgot to also skip scanning anonymous pages in that situation. That lead to the curious situation of keeping the active file pages protected from being paged out when there are lots of inactive file pages, while still scanning and evicting anonymous pages.

This patch fixes that situation, by only evicting file pages when we have plenty of them and most are inactive.

Signed-off-by Rik van Riel <[email protected]>

---
 mm/vmscan.c | 9 +++++++++
 1 file changed, 9 insertions(+)

mm/vmscan.c +9 -0

index 2624edc..1a53fbb 100644
@@ -1686,6 +1686,15 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 			fraction[1] = 0;
 			denominator = 1;
 			goto out;
+		} else if (!inactive_file_is_low_global(zone)) {
+			/*
+			 * There is enough inactive page cache, do not
+			 * reclaim anything from the working set right now.
+			 */
+			fraction[0] = 0;
+			fraction[1] = 1;
+			denominator = 1;
+			goto out;
 		}
 	}

--
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>

Andrew Morton

maintainer

2012-10-30 18:54 UTC #1 lore raw

Quoted 4 lines · On Tue, 30 Oct 2012 14:42:04 -0400 Rik van Riel <[email protected]> wrote:

If we have more inactive file pages than active file pages, we skip scanning the active file pages alltogether, with the idea that we do not want to evict the working set when there is plenty of streaming IO in the cache.

Yes, I've never liked that. The "(active > inactive)" thing is a magic number. And suddenly causing a complete cessation of vm scanning at a particular magic threshold seems rather crude, compared to some complex graduated thing which will also always do the wrong thing, only more obscurely ;)

Ho hum, in the absence of observed problems, I guess we don't muck with it.

Quoted 9 lines

However, the code forgot to also skip scanning anonymous pages in that situation. That lead to the curious situation of keeping the active file pages protected from being paged out when there are lots of inactive file pages, while still scanning and evicting anonymous pages.
This patch fixes that situation, by only evicting file pages when we have plenty of them and most are inactive.

Any observed runtime effects from this? If so, were they good?

Quoted 7 lines

mm/vmscan.c +1 -0

@@ -1686,6 +1686,15 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 			fraction[1] = 0;
 			denominator = 1;
 			goto out;
+		} else if (!inactive_file_is_low_global(zone)) {

mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 1699

Quoted 10 lines

mm/vmscan.c +8 -0

+			/*
+			 * There is enough inactive page cache, do not
+			 * reclaim anything from the working set right now.
+			 */
+			fraction[0] = 0;
+			fraction[1] = 1;
+			denominator = 1;
+			goto out;
 		}
 	}

Let's make the commenting look logical:

mm/vmscan.c renamed from mm/vmscan.c~mmvmscan-only-evict-file-pages-when-we-have-plenty-fix +4 -2

@@ -1679,9 +1679,11 @@ static void get_scan_count(struct lruvec
 
 	if (global_reclaim(sc)) {
 		free  = zone_page_state(zone, NR_FREE_PAGES);
-		/* If we have very few page cache pages,
-		   force-scan anon pages. */
 		if (unlikely(file + free <= high_wmark_pages(zone))) {
+			/*
+			 * If we have very few page cache pages, force-scan
+			 * anon pages.
+			 */
 			fraction[0] = 1;
 			fraction[1] = 0;
 			denominator = 1;

Rik van Riel

author

2012-10-30 19:06 UTC #2 lore raw

Quoted 8 lines · Andrew Morton

Quoted 4 lines · On Tue, 30 Oct 2012 14:42:04 -0400 Rik van Riel <[email protected]> wrote:
If we have more inactive file pages than active file pages, we skip scanning the active file pages alltogether, with the idea that we do not want to evict the working set when there is plenty of streaming IO in the cache.

Quoted 8 lines

Yes, I've never liked that. The "(active > inactive)" thing is a magic number. And suddenly causing a complete cessation of vm scanning at a particular magic threshold seems rather crude, compared to some complex graduated thing which will also always do the wrong thing, only more obscurely ;)
Ho hum, in the absence of observed problems, I guess we don't muck with it.

The thing is, when we "suddenly switch behaviour" back to scanning all the lists, that does not have to suddenly lead to pages from the other lists being actually evicted.

Instead, it will lead to referenced inactive_anon pages being moved back to the active_anon list, and any pages from the end of the active_file list being moved to the inactive_file list.

There is a threshold, and Johannes has patches to set the threshold in a much more intelligent way, but the change in behaviour should not be sudden due to the inactive lists providing a rather large buffer.

When the VM is bouncing around the threshold, it should look like a reduction in the rate at which the other lists are scanned.

Johannes Weiner

maintainer

2012-10-31 15:39 UTC #3 lore raw

In reply to Rik van Riel

Quoted 26 lines · Rik van Riel

If we have more inactive file pages than active file pages, we skip scanning the active file pages alltogether, with the idea that we do not want to evict the working set when there is plenty of streaming IO in the cache.
However, the code forgot to also skip scanning anonymous pages in that situation. That lead to the curious situation of keeping the active file pages protected from being paged out when there are lots of inactive file pages, while still scanning and evicting anonymous pages.
This patch fixes that situation, by only evicting file pages when we have plenty of them and most are inactive.
Signed-off-by Rik van Riel <[email protected]>
---
 mm/vmscan.c | 9 +++++++++
 1 file changed, 9 insertions(+)
mm/vmscan.c +0 -0
index 2624edc..1a53fbb 100644
@@ -1686,6 +1686,15 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 			fraction[1] = 0;
 			denominator = 1;

mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 1697

Quoted 10 lines

mm/vmscan.c +9 -0

 			goto out;
+		} else if (!inactive_file_is_low_global(zone)) {
+			/*
+			 * There is enough inactive page cache, do not
+			 * reclaim anything from the working set right now.
+			 */
+			fraction[0] = 0;
+			fraction[1] = 1;
+			denominator = 1;
+			goto out;

Is there a specific reason for making this exclusive to global reclaim? The "force scan anon when file is low" HAS to be specific to global reclaim because swapping may not be allowed in memcg limit reclaim, but not scanning anon when there is enough easy page cache is a legitimate memcg limit reclaim thing to do as well.

I.e. could this check be moved just below the

	/* If we have no swap space, do not bother scanning anon pages. */
	if (!sc->may_swap || (nr_swap_pages <= 0)) {
		noswap = 1;
		fraction[0] = 0;
		fraction[1] = 1;
		denominator = 1;
		goto out;
	}

section?

Rik van Riel

author

2012-10-31 16:14 UTC #4 lore raw

Quoted 35 lines · Johannes Weiner

Quoted 34 lines · Rik van Riel
If we have more inactive file pages than active file pages, we skip scanning the active file pages alltogether, with the idea that we do not want to evict the working set when there is plenty of streaming IO in the cache.
However, the code forgot to also skip scanning anonymous pages in that situation. That lead to the curious situation of keeping the active file pages protected from being paged out when there are lots of inactive file pages, while still scanning and evicting anonymous pages.
This patch fixes that situation, by only evicting file pages when we have plenty of them and most are inactive.
Signed-off-by Rik van Riel <[email protected]>
---
  mm/vmscan.c | 9 +++++++++
  1 file changed, 9 insertions(+)
mm/vmscan.c +7 -0
index 2624edc..1a53fbb 100644
@@ -1686,6 +1686,15 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
  			fraction[1] = 0;
  			denominator = 1;
  			goto out;
+		} else if (!inactive_file_is_low_global(zone)) {
+			/*
+			 * There is enough inactive page cache, do not
+			 * reclaim anything from the working set right now.
+			 */
+			fraction[0] = 0;
+			fraction[1] = 1;

Quoted 8 lines

mm/vmscan.c static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc, · line 1697
Quoted 2 lines
mm/vmscan.c +2 -0
+			denominator = 1;
+			goto out;
Is there a specific reason for making this exclusive to global reclaim? The "force scan anon when file is low" HAS to be specific to global reclaim because swapping may not be allowed in memcg limit reclaim, but not scanning anon when there is enough easy page cache is a legitimate memcg limit reclaim thing to do as well.

Good point. I guess this check would work fine inside cgroup reclaim, too.

Want to give that a try?

Quoted 12 lines

I.e. could this check be moved just below the

	/* If we have no swap space, do not bother scanning anon pages. */
	if (!sc->may_swap || (nr_swap_pages <= 0)) {
		noswap = 1;
		fraction[0] = 0;
		fraction[1] = 1;
		denominator = 1;
		goto out;
	}

Quoted 2 lines

section?