Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, Dec 19, 2012 at 12:21:55AM -0500, Simon Jeons wrote:
> 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
> > > ---
> > > 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
> > >
> > > 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.
>
Those comments were wrong.
--
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, Dec 19, 2012 at 12:21:55AM -0500, Simon Jeons wrote:
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 han...@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.
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 mho...@suse.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?
Hi Michal,
I confuse of Gorman's comments below, why the logic change still fine.
Those comments were wrong.
--
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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
> > ---
> > 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
> >
> > 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_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
Your patch
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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_highforce reclaim file
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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 majord...@vger.kernel.org
More majordomo info at
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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.
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 mho...@suse.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?
Hi Michal,
I confuse of Gorman's comments below, why the logic change still fine.
Current
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
Your patch
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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_highforce reclaim file
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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
> ---
> 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
>
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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 han...@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.
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 mho...@suse.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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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
---
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
Hi Michal,
I still can't understand why "The goto out means that it should be fine
either way.", could you explain to me, sorry for my stupid. :-)
Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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.
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 mho...@suse.cz
Hi Michal,
I still can't understand why The goto out means that it should be fine
either way., could you explain to me, sorry for my stupid. :-)
Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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
> > >---
> > > 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
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Thu, Dec 13, 2012 at 10:07:04AM +, Mel Gorman wrote: > On Wed, Dec 12, 2012 at 05:28:44PM -0500, 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 > > You are using some internal tree for that commit. Now that it's upstream > it is commit e9868505987a03a26a3979f27b82911ccc003752. > > > > >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 > > > >--- > > > > mm/vmscan.c | 19 ++- > > > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > > > > > > > > > > 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... :-) > > > > Does it actually make sense? Lets take the global reclaim case. > > I made a stupid mistake that Michal Hocko pointed out to me. The goto out means that it should be fine either way. > I'm not being super thorough because I'm not quite sure this is the right > patch if the motivation is for memcg to use the same logic. Instead of > moving this if, why do you not estimate "free" for the memcg based on the > hard limit and current usage? > I'm still curious about this part. -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, Dec 12, 2012 at 05:28:44PM -0500, 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
You are using some internal tree for that commit. Now that it's upstream
it is commit e9868505987a03a26a3979f27b82911ccc003752.
> > >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
> > >---
> > > 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... :-)
>
Does it actually make sense? Lets take the global reclaim case.
low_file == if (unlikely(file + free <= high_wmark_pages(zone)))
inactive_is_high == if (!inactive_file_is_low_global(zone))
Current
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
Your patch
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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_highforce reclaim file
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
There is a small but important change in results. I easily could have made
a mistake so double check.
I'm not being super thorough because I'm not quite sure this is the right
patch if the motivation is for memcg to use the same logic. Instead of
moving this if, why do you not estimate "free" for the memcg based on the
hard limit and current usage?
--
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, Dec 12, 2012 at 05:28:44PM -0500, 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
You are using some internal tree for that commit. Now that it's upstream
it is commit e9868505987a03a26a3979f27b82911ccc003752.
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 han...@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... :-)
Does it actually make sense? Lets take the global reclaim case.
low_file == if (unlikely(file + free = high_wmark_pages(zone)))
inactive_is_high == if (!inactive_file_is_low_global(zone))
Current
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
Your patch
low_file inactive_is_highforce reclaim anon
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce 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_highforce reclaim file
low_file !inactive_is_high force reclaim anon
!low_file inactive_is_highforce reclaim file
!low_file !inactive_is_high normal split
There is a small but important change in results. I easily could have made
a mistake so double check.
I'm not being super thorough because I'm not quite sure this is the right
patch if the motivation is for memcg to use the same logic. Instead of
moving this if, why do you not estimate free for the memcg based on the
hard limit and current usage?
--
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Thu, Dec 13, 2012 at 10:07:04AM +, Mel Gorman wrote: On Wed, Dec 12, 2012 at 05:28:44PM -0500, 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 You are using some internal tree for that commit. Now that it's upstream it is commit e9868505987a03a26a3979f27b82911ccc003752. 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 han...@cmpxchg.org --- mm/vmscan.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) SNIP 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... :-) Does it actually make sense? Lets take the global reclaim case. stupidity snipped I made a stupid mistake that Michal Hocko pointed out to me. The goto out means that it should be fine either way. I'm not being super thorough because I'm not quite sure this is the right patch if the motivation is for memcg to use the same logic. Instead of moving this if, why do you not estimate free for the memcg based on the hard limit and current usage? I'm still curious about this part. -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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.
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 mho...@suse.cz
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, 2012-12-12 at 16:53 -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
> > ---
> > 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.
Where check enough file pages?
if (unlikely(file + free <= high_wmark_pages(zone))), correct?
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, 2012-12-12 at 16:43 -0500, Johannes Weiner wrote:
> dc0422c "mm: vmscan: only evict file pages when we have plenty" makes
Can't find dc0422c.
> 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
> ---
> 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;
> }
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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
> >---
> > 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... :-)
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.
Thanks, Rik!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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
---
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 1/8] mm: memcg: only evict file pages when we have plenty
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
---
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;
}
}
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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;
}
}
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
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 han...@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... :-)
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.
Thanks, Rik!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, 2012-12-12 at 16:43 -0500, Johannes Weiner wrote:
dc0422c mm: vmscan: only evict file pages when we have plenty makes
Can't find dc0422c.
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 han...@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;
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
On Wed, 2012-12-12 at 16:53 -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 han...@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.
Where check enough file pages?
if (unlikely(file + free = high_wmark_pages(zone))), correct?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
