linux-kernel - Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 17 Dec 2012 16:54:16 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	Simon Jeons <simon.jeons@...il.com>
Cc:	Johannes Weiner <hannes@...xchg.org>,
	Rik van Riel <riel@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Hugh Dickins <hughd@...gle.com>,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty

On Sun 16-12-12 09:21:54, Simon Jeons wrote:
> On 12/13/2012 10:55 PM, Michal Hocko wrote:
> >On Wed 12-12-12 17:28:44, Johannes Weiner wrote:
> >>On Wed, Dec 12, 2012 at 04:53:36PM -0500, Rik van Riel wrote:
> >>>On 12/12/2012 04:43 PM, Johannes Weiner wrote:
> >>>>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@...xchg.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.
> >
> >>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 <mhocko@...e.cz>
> 
> 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/