All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fengguang Wu <fengguang.wu@intel.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujtisu.com>,
	Mel Gorman <mgorman@suse.de>, Minchan Kim <minchan@kernel.org>,
	Rik van Riel <riel@redhat.com>, Ying Han <yinghan@google.com>,
	Greg Thelen <gthelen@google.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [RFC -mm] memcg: prevent from OOM with too many dirty pages
Date: Tue, 29 May 2012 18:21:46 +0800	[thread overview]
Message-ID: <20120529102146.GA11653@localhost> (raw)
In-Reply-To: <20120529093511.GE1734@cmpxchg.org>

On Tue, May 29, 2012 at 11:35:11AM +0200, Johannes Weiner wrote:
> On Tue, May 29, 2012 at 04:48:48PM +0800, Fengguang Wu wrote:
> > On Tue, May 29, 2012 at 09:28:53AM +0200, Johannes Weiner wrote:
> > > On Tue, May 29, 2012 at 11:08:57AM +0800, Fengguang Wu wrote:
> > > > Hi Michal,
> > > > 
> > > > On Mon, May 28, 2012 at 05:38:55PM +0200, Michal Hocko wrote:
> > > > > Current implementation of dirty pages throttling is not memcg aware which makes
> > > > > it easy to have LRUs full of dirty pages which might lead to memcg OOM if the
> > > > > hard limit is small and so the lists are scanned faster than pages written
> > > > > back.
> > > > > 
> > > > > This patch fixes the problem by throttling the allocating process (possibly
> > > > > a writer) during the hard limit reclaim by waiting on PageReclaim pages.
> > > > > We are waiting only for PageReclaim pages because those are the pages
> > > > > that made one full round over LRU and that means that the writeback is much
> > > > > slower than scanning.
> > > > > The solution is far from being ideal - long term solution is memcg aware
> > > > > dirty throttling - but it is meant to be a band aid until we have a real
> > > > > fix.
> > > > 
> > > > IMHO it's still an important "band aid" -- perhaps worthwhile for
> > > > sending to Greg's stable trees. Because it fixes a really important
> > > > use case: it enables the users to put backups into a small memcg.
> > > > 
> > > > The users visible changes are:
> > > > 
> > > >         the backup program get OOM killed
> > > > =>
> > > >         it runs now, although being a bit slow and bumpy
> > > 
> > > The problem is workloads that /don't/ have excessive dirty pages, but
> > > instantiate clean page cache at a much faster rate than writeback can
> > > clean the few dirties.  The dirty/writeback pages reach the end of the
> > > lru several times while there are always easily reclaimable pages
> > > around.
> > 
> > Good point!
> > 
> > > This was the rationale for introducing the backoff function that
> > > considers the dirty page percentage of all pages looked at (bottom of
> > > shrink_active_list) and removing all other sleeps that didn't look at
> > > the bigger picture and made problems.  I'd hate for them to come back.
> > > 
> > > On the other hand, is there a chance to make this backoff function
> > > work for memcgs?  Right now it only applies to the global case to not
> > > mark a whole zone congested because of some dirty pages on a single
> > > memcg LRU.  But maybe it can work by considering congestion on a
> > > per-lruvec basis rather than per-zone?
> > 
> > Johannes, would you paste the backoff code? Sorry I'm not sure about
> > the exact logic you are talking.
> 
> Sure, it's this guy here:

Yeah I knew this code, but it's in shrink_inactive_list() ;)

>         /*
>          * If reclaim is isolating dirty pages under writeback, it implies
>          * that the long-lived page allocation rate is exceeding the page
>          * laundering rate. Either the global limits are not being effective
>          * at throttling processes due to the page distribution throughout
>          * zones or there is heavy usage of a slow backing device. The
>          * only option is to throttle from reclaim context which is not ideal
>          * as there is no guarantee the dirtying process is throttled in the
>          * same way balance_dirty_pages() manages.
>          *
>          * This scales the number of dirty pages that must be under writeback
>          * before throttling depending on priority. It is a simple backoff
>          * function that has the most effect in the range DEF_PRIORITY to
>          * DEF_PRIORITY-2 which is the priority reclaim is considered to be
>          * in trouble and reclaim is considered to be in trouble.
>          *
>          * DEF_PRIORITY   100% isolated pages must be PageWriteback to throttle
>          * DEF_PRIORITY-1  50% must be PageWriteback
>          * DEF_PRIORITY-2  25% must be PageWriteback, kswapd in trouble
>          * ...
>          * DEF_PRIORITY-6 For SWAP_CLUSTER_MAX isolated pages, throttle if any
>          *                     isolated page is PageWriteback
>          */
>         if (nr_writeback && nr_writeback >= (nr_taken >> (DEF_PRIORITY-priority)))
>                 wait_iff_congested(zone, BLK_RW_ASYNC, HZ/10);
> 
> But the problem is the part declaring the zone congested:
> 
>         /*
>          * Tag a zone as congested if all the dirty pages encountered were
>          * backed by a congested BDI. In this case, reclaimers should just
>          * back off and wait for congestion to clear because further reclaim
>          * will encounter the same problem
>          */
>         if (nr_dirty && nr_dirty == nr_congested && global_reclaim(sc))
>                 zone_set_flag(mz->zone, ZONE_CONGESTED);
> 
> Note the global_reclaim().  It would be nice to have these two operate
> against the lruvec of sc->target_mem_cgroup and mz->zone instead.  The
> problem is that ZONE_CONGESTED clearing happens in kswapd alone, which
> is not necessarily involved in a memcg-constrained load, so we need to
> find clearing sites that work for both global and memcg reclaim.

The problem of the above backoff logic is, both the conditions

>         if (nr_writeback && nr_writeback >= (nr_taken >> (DEF_PRIORITY-priority)))

and

>         if (nr_dirty && nr_dirty == nr_congested && global_reclaim(sc))

are based on local nr_writeback/nr_dirty values. "local" means inside
one SWAP_CLUSTER_MAX=32 batch. So if there is a continuous run of 32
dirty/writeback pages in the LRU, which is a common case even if there
are less than 20% dirty pages, the above conditions could accidentally
evaluate to true.

So in long term, we may consider the opposite way: to replace it with
the (PageReclaim && priority < X) test where the priority test is more
global wise.

For now, "priority" is not very stable. I often observe it being
knocked down to small values (eg. 5) due to the uneven distribution of
dirty pages over the LRU. But once we put dirty pages to a standalone
LRU list, "priority" will no longer come up and down that often, being
easily affected by the distribution of dirty pages.

> > As for this patch, can it be improved by adding some test like
> > (priority < DEF_PRIORITY/2)? That should reasonably filter out the
> > "fast read rotating dirty pages fast" situation and still avoid OOM
> > for "heavy write inside small memcg".
> 
> I think we tried these thresholds for global sync reclaim, too, but
> couldn't find the right value.  IIRC, we tried to strike a balance
> between excessive stalls and wasting CPU, but obviously the CPU
> wasting is not a concern because that is completely uninhibited right
> now for memcg reclaim.  So it may be an improvement if I didn't miss
> anything.  Maybe Mel remembers more?
> 
> It'd still be preferrable to keep the differences between memcg and
> global reclaim at a minimum, though, and extend the dirty throttling
> we already have.

Yeah we'll be introducing yet another magic value... Here we make
things simple by limiting the goal to avoid OOM in small memcg and
ignore other CPU/stall issues. For this target, it seems good to
choose a very low priority. For example, (priority < 3), which means
we've scanned 1/(2^2) = 25% dirty/writeback pages, which is slightly
larger than the 20% global dirty limit.

Thanks,
Fengguang

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Fengguang Wu <fengguang.wu@intel.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujtisu.com>,
	Mel Gorman <mgorman@suse.de>, Minchan Kim <minchan@kernel.org>,
	Rik van Riel <riel@redhat.com>, Ying Han <yinghan@google.com>,
	Greg Thelen <gthelen@google.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [RFC -mm] memcg: prevent from OOM with too many dirty pages
Date: Tue, 29 May 2012 18:21:46 +0800	[thread overview]
Message-ID: <20120529102146.GA11653@localhost> (raw)
In-Reply-To: <20120529093511.GE1734@cmpxchg.org>

On Tue, May 29, 2012 at 11:35:11AM +0200, Johannes Weiner wrote:
> On Tue, May 29, 2012 at 04:48:48PM +0800, Fengguang Wu wrote:
> > On Tue, May 29, 2012 at 09:28:53AM +0200, Johannes Weiner wrote:
> > > On Tue, May 29, 2012 at 11:08:57AM +0800, Fengguang Wu wrote:
> > > > Hi Michal,
> > > > 
> > > > On Mon, May 28, 2012 at 05:38:55PM +0200, Michal Hocko wrote:
> > > > > Current implementation of dirty pages throttling is not memcg aware which makes
> > > > > it easy to have LRUs full of dirty pages which might lead to memcg OOM if the
> > > > > hard limit is small and so the lists are scanned faster than pages written
> > > > > back.
> > > > > 
> > > > > This patch fixes the problem by throttling the allocating process (possibly
> > > > > a writer) during the hard limit reclaim by waiting on PageReclaim pages.
> > > > > We are waiting only for PageReclaim pages because those are the pages
> > > > > that made one full round over LRU and that means that the writeback is much
> > > > > slower than scanning.
> > > > > The solution is far from being ideal - long term solution is memcg aware
> > > > > dirty throttling - but it is meant to be a band aid until we have a real
> > > > > fix.
> > > > 
> > > > IMHO it's still an important "band aid" -- perhaps worthwhile for
> > > > sending to Greg's stable trees. Because it fixes a really important
> > > > use case: it enables the users to put backups into a small memcg.
> > > > 
> > > > The users visible changes are:
> > > > 
> > > >         the backup program get OOM killed
> > > > =>
> > > >         it runs now, although being a bit slow and bumpy
> > > 
> > > The problem is workloads that /don't/ have excessive dirty pages, but
> > > instantiate clean page cache at a much faster rate than writeback can
> > > clean the few dirties.  The dirty/writeback pages reach the end of the
> > > lru several times while there are always easily reclaimable pages
> > > around.
> > 
> > Good point!
> > 
> > > This was the rationale for introducing the backoff function that
> > > considers the dirty page percentage of all pages looked at (bottom of
> > > shrink_active_list) and removing all other sleeps that didn't look at
> > > the bigger picture and made problems.  I'd hate for them to come back.
> > > 
> > > On the other hand, is there a chance to make this backoff function
> > > work for memcgs?  Right now it only applies to the global case to not
> > > mark a whole zone congested because of some dirty pages on a single
> > > memcg LRU.  But maybe it can work by considering congestion on a
> > > per-lruvec basis rather than per-zone?
> > 
> > Johannes, would you paste the backoff code? Sorry I'm not sure about
> > the exact logic you are talking.
> 
> Sure, it's this guy here:

Yeah I knew this code, but it's in shrink_inactive_list() ;)

>         /*
>          * If reclaim is isolating dirty pages under writeback, it implies
>          * that the long-lived page allocation rate is exceeding the page
>          * laundering rate. Either the global limits are not being effective
>          * at throttling processes due to the page distribution throughout
>          * zones or there is heavy usage of a slow backing device. The
>          * only option is to throttle from reclaim context which is not ideal
>          * as there is no guarantee the dirtying process is throttled in the
>          * same way balance_dirty_pages() manages.
>          *
>          * This scales the number of dirty pages that must be under writeback
>          * before throttling depending on priority. It is a simple backoff
>          * function that has the most effect in the range DEF_PRIORITY to
>          * DEF_PRIORITY-2 which is the priority reclaim is considered to be
>          * in trouble and reclaim is considered to be in trouble.
>          *
>          * DEF_PRIORITY   100% isolated pages must be PageWriteback to throttle
>          * DEF_PRIORITY-1  50% must be PageWriteback
>          * DEF_PRIORITY-2  25% must be PageWriteback, kswapd in trouble
>          * ...
>          * DEF_PRIORITY-6 For SWAP_CLUSTER_MAX isolated pages, throttle if any
>          *                     isolated page is PageWriteback
>          */
>         if (nr_writeback && nr_writeback >= (nr_taken >> (DEF_PRIORITY-priority)))
>                 wait_iff_congested(zone, BLK_RW_ASYNC, HZ/10);
> 
> But the problem is the part declaring the zone congested:
> 
>         /*
>          * Tag a zone as congested if all the dirty pages encountered were
>          * backed by a congested BDI. In this case, reclaimers should just
>          * back off and wait for congestion to clear because further reclaim
>          * will encounter the same problem
>          */
>         if (nr_dirty && nr_dirty == nr_congested && global_reclaim(sc))
>                 zone_set_flag(mz->zone, ZONE_CONGESTED);
> 
> Note the global_reclaim().  It would be nice to have these two operate
> against the lruvec of sc->target_mem_cgroup and mz->zone instead.  The
> problem is that ZONE_CONGESTED clearing happens in kswapd alone, which
> is not necessarily involved in a memcg-constrained load, so we need to
> find clearing sites that work for both global and memcg reclaim.

The problem of the above backoff logic is, both the conditions

>         if (nr_writeback && nr_writeback >= (nr_taken >> (DEF_PRIORITY-priority)))

and

>         if (nr_dirty && nr_dirty == nr_congested && global_reclaim(sc))

are based on local nr_writeback/nr_dirty values. "local" means inside
one SWAP_CLUSTER_MAX=32 batch. So if there is a continuous run of 32
dirty/writeback pages in the LRU, which is a common case even if there
are less than 20% dirty pages, the above conditions could accidentally
evaluate to true.

So in long term, we may consider the opposite way: to replace it with
the (PageReclaim && priority < X) test where the priority test is more
global wise.

For now, "priority" is not very stable. I often observe it being
knocked down to small values (eg. 5) due to the uneven distribution of
dirty pages over the LRU. But once we put dirty pages to a standalone
LRU list, "priority" will no longer come up and down that often, being
easily affected by the distribution of dirty pages.

> > As for this patch, can it be improved by adding some test like
> > (priority < DEF_PRIORITY/2)? That should reasonably filter out the
> > "fast read rotating dirty pages fast" situation and still avoid OOM
> > for "heavy write inside small memcg".
> 
> I think we tried these thresholds for global sync reclaim, too, but
> couldn't find the right value.  IIRC, we tried to strike a balance
> between excessive stalls and wasting CPU, but obviously the CPU
> wasting is not a concern because that is completely uninhibited right
> now for memcg reclaim.  So it may be an improvement if I didn't miss
> anything.  Maybe Mel remembers more?
> 
> It'd still be preferrable to keep the differences between memcg and
> global reclaim at a minimum, though, and extend the dirty throttling
> we already have.

Yeah we'll be introducing yet another magic value... Here we make
things simple by limiting the goal to avoid OOM in small memcg and
ignore other CPU/stall issues. For this target, it seems good to
choose a very low priority. For example, (priority < 3), which means
we've scanned 1/(2^2) = 25% dirty/writeback pages, which is slightly
larger than the 20% global dirty limit.

Thanks,
Fengguang

  reply	other threads:[~2012-05-29 10:22 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-28 15:38 [RFC -mm] memcg: prevent from OOM with too many dirty pages Michal Hocko
2012-05-28 15:38 ` Michal Hocko
2012-05-29  3:08 ` Fengguang Wu
2012-05-29  3:08   ` Fengguang Wu
2012-05-29  7:28   ` Johannes Weiner
2012-05-29  7:28     ` Johannes Weiner
2012-05-29  8:48     ` Fengguang Wu
2012-05-29  8:48       ` Fengguang Wu
2012-05-29  9:35       ` Johannes Weiner
2012-05-29  9:35         ` Johannes Weiner
2012-05-29 10:21         ` Fengguang Wu [this message]
2012-05-29 10:21           ` Fengguang Wu
2012-05-29 13:32         ` Mel Gorman
2012-05-29 13:32           ` Mel Gorman
2012-05-29 13:51         ` Michal Hocko
2012-05-29 13:51           ` Michal Hocko
2012-05-31  9:09           ` Michal Hocko
2012-06-01  8:37             ` Michal Hocko
2012-06-01  8:37               ` Michal Hocko
2012-06-07 14:45               ` Michal Hocko
2012-06-07 14:45                 ` Michal Hocko
2012-06-14  7:27                 ` Johannes Weiner
2012-06-14  7:27                   ` Johannes Weiner
2012-06-14 10:13                   ` Michal Hocko
2012-06-14 10:13                     ` Michal Hocko
2012-05-31 15:18           ` Fengguang Wu
     [not found]             ` <20120531153249.GD12809@tiehlicka.suse.cz>
     [not found]               ` <20120531154248.GA32734@localhost>
     [not found]                 ` <20120531154859.GA20546@tiehlicka.suse.cz>
     [not found]                   ` <20120531160129.GA439@localhost>
     [not found]                     ` <20120531182509.GA22539@tiehlicka.suse.cz>
2012-06-01  1:33                       ` Fengguang Wu
2012-06-01  1:33                         ` Fengguang Wu

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=20120529102146.GA11653@localhost \
    --to=fengguang.wu@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=gthelen@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kamezawa.hiroyu@jp.fujtisu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    --cc=yinghan@google.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.