[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160725092909.GV11400@suse.de>
Date: Mon, 25 Jul 2016 10:29:09 +0100
From: Mel Gorman <mgorman@...e.de>
To: Minchan Kim <minchan@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC] mm: bail out in shrin_inactive_list
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.
> 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.
> 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
Powered by blists - more mailing lists