linux-kernel - Re: [RFC] mm: bail out in shrin_inactive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160726012157.GA11651@bbox>
Date:	Tue, 26 Jul 2016 10:21:57 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Mel Gorman <mgorman@...e.de>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>, Michal Hocko <mhocko@...nel.org>,
	Vladimir Davydov <vdavydov@...tuozzo.com>
Subject: Re: [RFC] mm: bail out in shrin_inactive_list

On Mon, Jul 25, 2016 at 10:29:09AM +0100, Mel Gorman wrote:
> There is a typo in the subject line.
> 
> On Mon, Jul 25, 2016 at 04:51:59PM +0900, Minchan Kim wrote:
> > With node-lru, if there are enough reclaimable pages in highmem
> > but nothing in lowmem, VM can try to shrink inactive list although
> > the requested zone is lowmem.
> > 
> > The problem is direct reclaimer scans inactive list is fulled with
> 
> 
> > highmem pages to find a victim page at a reqested zone or lower zones
> > but the result is that VM should skip all of pages. 
> 
> Rephease -- The problem is that if the inactive list is full of highmem
> pages then a direct reclaimer searching for a lowmem page waste CPU
> scanning uselessly.

It's better. Thanks.

> 
> > CPU. Even, many direct reclaimers are stalled by too_many_isolated
> > if lots of parallel reclaimer are going on although there are no
> > reclaimable memory in inactive list.
> > 
> > I tried the experiment 4 times in 32bit 2G 8 CPU KVM machine
> > to get elapsed time.
> > 
> > 	hackbench 500 process 2
> > 
> > = Old =
> > 
> > 1st: 289s 2nd: 310s 3rd: 112s 4th: 272s
> > 
> > = Now =
> > 
> > 1st: 31s  2nd: 132s 3rd: 162s 4th: 50s.
> > 
> > Signed-off-by: Minchan Kim <minchan@...nel.org>
> > ---
> > I believe proper fix is to modify get_scan_count. IOW, I think
> > we should introduce lruvec_reclaimable_lru_size with proper
> > classzone_idx but I don't know how we can fix it with memcg
> > which doesn't have zone stat now. should introduce zone stat
> > back to memcg? Or, it's okay to ignore memcg?
> > 
> 
> I think it's ok to ignore memcg in this case as a memcg shrink is often
> going to be for pages that can use highmem anyway.

So, you mean it's okay to ignore kmemcg case?
If memcg guys agree it, I want to make get_scan_count consider
reclaimable lru size under the reclaim constraint, instead.

> 
> >  mm/vmscan.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index e5af357..3d285cc 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1652,6 +1652,31 @@ static int current_may_throttle(void)
> >  		bdi_write_congested(current->backing_dev_info);
> >  }
> >  
> > +static inline bool inactive_reclaimable_pages(struct lruvec *lruvec,
> > +				struct scan_control *sc,
> > +				enum lru_list lru)
> 
> inline is unnecessary. The function is long but only has one caller so
> it'll be inlined automatically.
> 
> > +{
> > +	int zid;
> > +	struct zone *zone;
> > +	bool file = is_file_lru(lru);
> 
> It's more appropriate to use int for file in this case as it's used as a
> multiplier. It'll work either way.
> 
> Otherwise;
> 
> Acked-by: Mel Gorman <mgorman@...hsingularity.net>
> 
> -- 
> Mel Gorman
> SUSE Labs