On Mon, 2012-12-17 at 16:54 +0100, Michal Hocko wrote: > 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 <[email protected]> > > >>>>--- > > >>>> 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 <[email protected]> > > > > 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?
Hi Michal, I confuse of Gorman's comments below, why the logic change still fine. Current low_file inactive_is_high force reclaim anon low_file !inactive_is_high force reclaim anon !low_file inactive_is_high force reclaim file !low_file !inactive_is_high normal split Your patch low_file inactive_is_high force reclaim anon low_file !inactive_is_high force reclaim anon !low_file inactive_is_high force reclaim file !low_file !inactive_is_high normal split However, if you move the inactive_file_is_low check down you get Moving the check low_file inactive_is_high force reclaim file low_file !inactive_is_high force reclaim anon !low_file inactive_is_high force reclaim file !low_file !inactive_is_high normal split > > > could you explain to me, sorry for my stupid. :-) > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
