All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Rik van Riel <riel@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>, Hugh Dickins <hughd@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
Date: Thu, 13 Dec 2012 14:44:38 +0000	[thread overview]
Message-ID: <20121213144438.GC9887@suse.de> (raw)
In-Reply-To: <20121213100704.GV1009@suse.de>

On Thu, Dec 13, 2012 at 10:07:04AM +0000, 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 <hannes@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

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Rik van Riel <riel@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>, Hugh Dickins <hughd@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 1/8] mm: memcg: only evict file pages when we have plenty
Date: Thu, 13 Dec 2012 14:44:38 +0000	[thread overview]
Message-ID: <20121213144438.GC9887@suse.de> (raw)
In-Reply-To: <20121213100704.GV1009@suse.de>

On Thu, Dec 13, 2012 at 10:07:04AM +0000, 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 <hannes@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, 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>

  reply	other threads:[~2012-12-13 14:44 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-12 21:43 [patch 0/8] page reclaim bits Johannes Weiner
2012-12-12 21:43 ` Johannes Weiner
2012-12-12 21:43 ` [patch 1/8] mm: memcg: only evict file pages when we have plenty Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 21:53   ` Rik van Riel
2012-12-12 21:53     ` Rik van Riel
2012-12-12 22:28     ` Johannes Weiner
2012-12-12 22:28       ` Johannes Weiner
2012-12-13 10:07       ` Mel Gorman
2012-12-13 10:07         ` Mel Gorman
2012-12-13 14:44         ` Mel Gorman [this message]
2012-12-13 14:44           ` Mel Gorman
2012-12-13 14:55       ` Michal Hocko
2012-12-13 14:55         ` Michal Hocko
2012-12-16  1:21         ` Simon Jeons
2012-12-16  1:21           ` Simon Jeons
2012-12-17 15:54           ` Michal Hocko
2012-12-17 15:54             ` Michal Hocko
2012-12-19  5:21             ` Simon Jeons
2012-12-19  5:21               ` Simon Jeons
2012-12-19  9:20               ` Mel Gorman
2012-12-19  9:20                 ` Mel Gorman
2012-12-13  5:36     ` Simon Jeons
2012-12-13  5:36       ` Simon Jeons
2012-12-13  5:34   ` Simon Jeons
2012-12-13  5:34     ` Simon Jeons
2012-12-12 21:43 ` [patch 2/8] mm: vmscan: disregard swappiness shortly before going OOM Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:01   ` Rik van Riel
2012-12-12 22:01     ` Rik van Riel
2012-12-13  5:56   ` Simon Jeons
2012-12-13  5:56     ` Simon Jeons
2012-12-13 10:34   ` Mel Gorman
2012-12-13 10:34     ` Mel Gorman
2012-12-13 15:29     ` Michal Hocko
2012-12-13 15:29       ` Michal Hocko
2012-12-13 16:05       ` Michal Hocko
2012-12-13 16:05         ` Michal Hocko
2012-12-13 22:25         ` Satoru Moriya
2012-12-13 22:25           ` Satoru Moriya
2012-12-14  4:50           ` Johannes Weiner
2012-12-14  4:50             ` Johannes Weiner
2012-12-14  8:37             ` Michal Hocko
2012-12-14  8:37               ` Michal Hocko
2012-12-14 15:43               ` Rik van Riel
2012-12-14 15:43                 ` Rik van Riel
2012-12-14 16:13                 ` Michal Hocko
2012-12-14 16:13                   ` Michal Hocko
2012-12-15  0:18                   ` Johannes Weiner
2012-12-15  0:18                     ` Johannes Weiner
2012-12-17 16:37                     ` Michal Hocko
2012-12-17 16:37                       ` Michal Hocko
2012-12-17 17:54                       ` Johannes Weiner
2012-12-17 17:54                         ` Johannes Weiner
2012-12-17 19:58                         ` Michal Hocko
2012-12-17 19:58                           ` Michal Hocko
2012-12-14 20:17                 ` Satoru Moriya
2012-12-14 20:17                   ` Satoru Moriya
2012-12-14 19:44               ` Satoru Moriya
2012-12-14 19:44                 ` Satoru Moriya
2012-12-13 19:05     ` Johannes Weiner
2012-12-13 19:05       ` Johannes Weiner
2012-12-13 19:47       ` Mel Gorman
2012-12-13 19:47         ` Mel Gorman
2012-12-12 21:43 ` [patch 3/8] mm: vmscan: save work scanning (almost) empty LRU lists Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:02   ` Rik van Riel
2012-12-12 22:02     ` Rik van Riel
2012-12-13 10:41   ` Mel Gorman
2012-12-13 10:41     ` Mel Gorman
2012-12-13 19:33     ` Johannes Weiner
2012-12-13 19:33       ` Johannes Weiner
2012-12-13 15:43   ` Michal Hocko
2012-12-13 15:43     ` Michal Hocko
2012-12-13 19:38     ` Johannes Weiner
2012-12-13 19:38       ` Johannes Weiner
2012-12-14  8:46       ` Michal Hocko
2012-12-14  8:46         ` Michal Hocko
2012-12-12 21:43 ` [patch 4/8] mm: vmscan: clarify LRU balancing close to OOM Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:03   ` Rik van Riel
2012-12-12 22:03     ` Rik van Riel
2012-12-13 10:46   ` Mel Gorman
2012-12-13 10:46     ` Mel Gorman
2012-12-12 21:43 ` [patch 5/8] mm: vmscan: improve comment on low-page cache handling Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:04   ` Rik van Riel
2012-12-12 22:04     ` Rik van Riel
2012-12-13 10:47   ` Mel Gorman
2012-12-13 10:47     ` Mel Gorman
2012-12-13 16:07   ` Michal Hocko
2012-12-13 16:07     ` Michal Hocko
2012-12-12 21:43 ` [patch 6/8] mm: vmscan: clean up get_scan_count() Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:06   ` Rik van Riel
2012-12-12 22:06     ` Rik van Riel
2012-12-13 11:07   ` Mel Gorman
2012-12-13 11:07     ` Mel Gorman
2012-12-13 16:18   ` Michal Hocko
2012-12-13 16:18     ` Michal Hocko
2012-12-12 21:43 ` [patch 7/8] mm: vmscan: compaction works against zones, not lruvecs Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:31   ` Rik van Riel
2012-12-12 22:31     ` Rik van Riel
2012-12-13 11:12   ` Mel Gorman
2012-12-13 11:12     ` Mel Gorman
2012-12-13 16:48   ` Michal Hocko
2012-12-13 16:48     ` Michal Hocko
2012-12-12 21:43 ` [patch 8/8] mm: reduce rmap overhead for ex-KSM page copies created on swap faults Johannes Weiner
2012-12-12 21:43   ` Johannes Weiner
2012-12-12 22:34   ` Rik van Riel
2012-12-12 22:34     ` Rik van Riel
2012-12-12 21:50 ` [patch 0/8] page reclaim bits Andrew Morton
2012-12-12 21:50   ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121213144438.GC9887@suse.de \
    --to=mgorman@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.