On Sun 16-12-12 09:21:54, Simon Jeons wrote:

On 12/13/2012 10:55 PM, Michal Hocko wrote:

quoted

On Wed 12-12-12 17:28:44, Johannes Weiner wrote:

quoted

On Wed, Dec 12, 2012 at 04:53:36PM -0500, Rik van Riel wrote:

quoted

On 12/12/2012 04:43 PM, Johannes Weiner wrote:

quoted

dc0422c "mm: vmscan: only evict file pages when we have plenty" makes
a point of not going for anonymous memory while there is still enough
inactive cache around.

The check was added only for global reclaim, but it is just as useful
for memory cgroup reclaim.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/vmscan.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 157bb11..3874dcb 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c

@@ -1671,6 +1671,16 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 		denominator = 1;
 		goto out;
 	}
+	/*
+	 * There is enough inactive page cache, do not reclaim
+	 * anything from the anonymous working set right now.
+	 */
+	if (!inactive_file_is_low(lruvec)) {
+		fraction[0] = 0;
+		fraction[1] = 1;
+		denominator = 1;
+		goto out;
+	}

 	anon  = get_lru_size(lruvec, LRU_ACTIVE_ANON) +
 		get_lru_size(lruvec, LRU_INACTIVE_ANON);

@@ -1688,15 +1698,6 @@ 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;
 		}
 	}

I believe the if() block should be moved to AFTER
the check where we make sure we actually have enough
file pages.

You are absolutely right, this makes more sense.  Although I'd figure
the impact would be small because if there actually is that little
file cache, it won't be there for long with force-file scanning... :-)

Yes, I think that the result would be worse (more swapping) so the
change can only help.

quoted

I moved the condition, but it throws conflicts in the rest of the
series.  Will re-run tests, wait for Michal and Mel, then resend.

Yes the patch makes sense for memcg as well. I guess you have tested
this primarily with memcg. Do you have any numbers? Would be nice to put
them into the changelog if you have (it should help to reduce swapping
with heavy streaming IO load).

Acked-by: Michal Hocko <redacted>

Hi Michal,

I still can't understand why "The goto out means that it should be
fine either way.",

Not sure I understand your question. goto out just says that either page
cache is low or inactive file LRU is too small. And it works for both
memcg and global because the page cache is low condition is evaluated
only for the global reclaim and always before inactive file is small.
Makes more sense?

could you explain to me, sorry for my stupid. :-)

-- 
Michal Hocko
SUSE Labs

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