Re: [PATCH v6] memcg: event control at vmpressure.

From: Hyunhee Kim <hyunhee.kim@samsung.com>
To: Anton Vorontsov <anton@enomsg.org>
Cc: Michal Hocko <mhocko@suse.cz>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	hannes@cmpxchg.org, rob@landley.net, linux-mm@kvack.org,
	rientjes@google.com, Minchan Kim <minchan@kernel.org>,
	kirill@shutemov.name, akpm@linux-foundation.org,
	kamezawa.hiroyu@jp.fujitsu.com
Subject: Re: [PATCH v6] memcg: event control at vmpressure.
Date: Sat, 22 Jun 2013 10:28:02 +0900	[thread overview]
Message-ID: <CAOK=xRMzZkk-r1TXRgdk-tZ+6AvXGij511y=PFcw5Smnt1rOHw@mail.gmail.com> (raw)
In-Reply-To: <20130622002744.GA29172@lizard.mcd26095.sjc.wayport.net>

2013. 6. 22. 오전 9:27에 "Anton Vorontsov" <anton@enomsg.org>님이 작성:
>
> On Sat, Jun 22, 2013 at 01:44:14AM +0900, Minchan Kim wrote:
> [...]
> > 3. The reclaimed could be greater than scanned in vmpressure_evnet
> >    by several reasons. Totally, It could trigger wrong event.
>
> Yup, and in that case the best we can do is just ignore the event (i.e.
> not pass it to the userland): thing is, based on the fact that
> 'reclaimed > scanned' we can't actually conclude anything about the
> pressure: it might be still high, or we actually freed enough.
>
> Thanks,
>
> Anton
>
> p.s. I was somewhat sure that someone sent a patch to ignore 'reclaimed >
> scanned' situation, but I cannot find it in my mailbox. Maybe I was
> dreaming about it? :)

I have suggested it as follows and Minchan reviewed it.
I'll send it again after applying Minchan's opinion.
Thanks,

Hyunhee Kim.
=========================================

On Wed, Jun 05, 2013 at 05:31:30PM +0900, Hyunhee Kim wrote:
 > Hi, Anton,
 >

Sorry, I'm not Anton but I involved a little bit when this feature was
 developed so may I answer your qeustion?

> When calculating pressure level in vmpressure_calc_level, I observed that "reclaimed" becomes larger than "scanned".
 > In this case, since these values are "unsigned long", pressure
returns wrong value and critical event is triggered even on low state.
 > Do you think that it is possible?

True, we have a few reasons.

Culprits I can think easily are THP page reclaiming or bails out reclaiming
 by fatal signal in shrink_inactive_list.
 I guess you don't enable THP so I think culprit is latter.

> If so, in this case, should we make "reclaimed" equal to "scanned"?
 > When I tested as below, it could trigger reasonable events.
 >
 > =============================
 > +static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 > +                                                 unsigned long reclaimed)
 > +{
 > +     unsigned long scale = scanned + reclaimed;
 > +     unsigned long pressure;
 > +     if (reclaimed > scanned)
 > +             reclaimed = scanned;

Could we simply return VMPRESSURE_LOW?
 ========================================

> --
> 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>

--
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>