lkml.org 
[lkml]   [2012]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC -mm] memcg: prevent from OOM with too many dirty pages
[restore CC list]

On Thu, May 31, 2012 at 08:25:09PM +0200, Michal Hocko wrote:
> On Fri 01-06-12 00:01:29, Fengguang Wu wrote:
> > On Thu, May 31, 2012 at 05:49:00PM +0200, Michal Hocko wrote:
> > > On Thu 31-05-12 23:42:48, Fengguang Wu wrote:
> > > > On Thu, May 31, 2012 at 05:32:49PM +0200, Michal Hocko wrote:
> > > > > JFYI: You might have missed https://lkml.org/lkml/2012/5/31/122 because
> > > > > intel mail server returned with "we do not like shell scripts as a
> > > > > attachment".
> > > >
> > > > Yeah I did miss it.. A quick question: does the PageReclaim patch
> > > > tested include the (priority < 3) test, or it's the originally posted
> > > > patch?
> > >
> > > It's the original patch.
> >
> > OK, that's fine. I suspect even adding (priority < 3), it can help
> > only some situations. In the others, the effect will be that writeback
> > pages get accumulated in LRU until it starts to throttle page reclaims
> > from both reads/writes. There will have to be some throttling somewhere.
>
> Yes, I agree. But it could help at least sporadic "hey this is a PageReclaim"
> issues. I just didn't like to push priority into shrink_page_list
> without. The justification is quite hard.

Yeah priority is just a rule of thumb "there may be lots of
dirty/writeback pages or other pressure if priority goes low".
And it's already been used this way in shrink_page_list().

Considering that it's also targeting for -stable merge, we do need
a very strict condition to safeguard no regressions on other cases.
This is also true for the wait_iff_congested() scheme.

> > Subject: mm: pass __GFP_WRITE to memcg charge and reclaim routines
> >
> > __GFP_WRITE will be tested in vmscan to find out the write tasks.
> >
> > For good interactive performance, we try to focus dirty reclaim waits on
> > them and avoid blocking unrelated tasks.
> >
> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>
> I will have a look at this one tomorrow with a clean head.

OK. The usage in my mind is

if (PageWriteback(page) && PageReclaim(page))
+ if ((sc->gfp_mask & __GFP_WAIT) || (priority < 3))
do some dirty throttling
But note that it only detects writes to new pages (ie. simple dd).
Overwrites to already cached clean pages cannot be detected this way..

Thanks,
Fengguang

> > ---
> > include/linux/gfp.h | 2 +-
> > mm/filemap.c | 17 ++++++++++-------
> > 2 files changed, 11 insertions(+), 8 deletions(-)
> >
> > --- linux.orig/include/linux/gfp.h 2012-03-02 14:06:47.501765703 +0800
> > +++ linux/include/linux/gfp.h 2012-03-02 14:07:39.921766949 +0800
> > @@ -129,7 +129,7 @@ struct vm_area_struct;
> > /* Control page allocator reclaim behavior */
> > #define GFP_RECLAIM_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO|__GFP_FS|\
> > __GFP_NOWARN|__GFP_REPEAT|__GFP_NOFAIL|\
> > - __GFP_NORETRY|__GFP_NOMEMALLOC)
> > + __GFP_NORETRY|__GFP_NOMEMALLOC|__GFP_WRITE)
> >
> > /* Control slab gfp mask during early boot */
> > #define GFP_BOOT_MASK (__GFP_BITS_MASK & ~(__GFP_WAIT|__GFP_IO|__GFP_FS))
> > --- linux.orig/mm/filemap.c 2012-03-02 14:07:21.000000000 +0800
> > +++ linux/mm/filemap.c 2012-03-02 14:07:53.709767277 +0800
> > @@ -2339,23 +2339,26 @@ struct page *grab_cache_page_write_begin
> > int status;
> > gfp_t gfp_mask;
> > struct page *page;
> > - gfp_t gfp_notmask = 0;
> > + gfp_t lru_gfp_mask = GFP_KERNEL;
> >
> > gfp_mask = mapping_gfp_mask(mapping);
> > - if (mapping_cap_account_dirty(mapping))
> > + if (mapping_cap_account_dirty(mapping)) {
> > gfp_mask |= __GFP_WRITE;
> > - if (flags & AOP_FLAG_NOFS)
> > - gfp_notmask = __GFP_FS;
> > + lru_gfp_mask |= __GFP_WRITE;
> > + }
> > + if (flags & AOP_FLAG_NOFS) {
> > + gfp_mask &= ~__GFP_FS;
> > + lru_gfp_mask &= ~__GFP_FS;
> > + }
> > repeat:
> > page = find_lock_page(mapping, index);
> > if (page)
> > goto found;
> >
> > - page = __page_cache_alloc(gfp_mask & ~gfp_notmask);
> > + page = __page_cache_alloc(gfp_mask);
> > if (!page)
> > return NULL;
> > - status = add_to_page_cache_lru(page, mapping, index,
> > - GFP_KERNEL & ~gfp_notmask);
> > + status = add_to_page_cache_lru(page, mapping, index, lru_gfp_mask);
> > if (unlikely(status)) {
> > page_cache_release(page);
> > if (status == -EEXIST)
>
>
> --
> Michal Hocko
> SUSE Labs
> SUSE LINUX s.r.o.
> Lihovarska 1060/12
> 190 00 Praha 9
> Czech Republic


\
 
 \ /
  Last update: 2012-06-01 04:01    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site