Date:	Tue, 29 May 2012 18:21:46 +0800
From:	Fengguang Wu <fengguang.wu@...el.com>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Michal Hocko <mhocko@...e.cz>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujtisu.com>,
	Mel Gorman <mgorman@...e.de>, Minchan Kim <minchan@...nel.org>,
	Rik van Riel <riel@...hat.com>,
	Ying Han <yinghan@...gle.com>,
	Greg Thelen <gthelen@...gle.com>,
	Hugh Dickins <hughd@...gle.com>
Subject: Re: [RFC -mm] memcg: prevent from OOM with too many dirty pages

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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists