All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"ming.ling" <ming.ling@spreadtrum.com>,
	Minchan Kim <minchan@kernel.org>, Mel Gorman <mgorman@suse.de>,
	Joonsoo Kim <js1304@gmail.com>,
	linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm, compaction: fix NR_ISOLATED_* stats for pfn based migration
Date: Wed, 19 Oct 2016 12:36:16 +0200	[thread overview]
Message-ID: <20161019103616.GG7517@dhcp22.suse.cz> (raw)
In-Reply-To: <2e4d79f9-74e5-5085-4037-caa9c1cb43e4@suse.cz>

On Wed 19-10-16 11:39:36, Vlastimil Babka wrote:
> On 10/19/2016 10:02 AM, Michal Hocko wrote:
> > From: Ming Ling <ming.ling@spreadtrum.com>
> > 
> > Since bda807d44454 ("mm: migrate: support non-lru movable page
> > migration") isolate_migratepages_block) can isolate !PageLRU pages which
> > would acct_isolated account as NR_ISOLATED_*. Accounting these non-lru
> > pages NR_ISOLATED_{ANON,FILE} doesn't make any sense and it can misguide
> > heuristics based on those counters such as pgdat_reclaimable_pages resp.
> > too_many_isolated which would lead to unexpected stalls during the
> > direct reclaim without any good reason. Note that
> > __alloc_contig_migrate_range can isolate a lot of pages at once.
> > 
> > On mobile devices such as 512M ram android Phone, it may use a big zram
> > swap. In some cases zram(zsmalloc) uses too many non-lru but migratedable
> > pages, such as:
> > 
> >       MemTotal: 468148 kB
> >       Normal free:5620kB
> >       Free swap:4736kB
> >       Total swap:409596kB
> >       ZRAM: 164616kB(zsmalloc non-lru pages)
> >       active_anon:60700kB
> >       inactive_anon:60744kB
> >       active_file:34420kB
> >       inactive_file:37532kB
> > 
> > Fix this by only accounting lru pages to NR_ISOLATED_* in
> > isolate_migratepages_block right after they were isolated and we still
> > know they were on LRU. Drop acct_isolated because it is called after the
> > fact and we've lost that information. Batching per-cpu counter doesn't
> > make much improvement anyway. Also make sure that we uncharge only LRU
> > pages when putting them back on the LRU in putback_movable_pages resp.
> > when unmap_and_move migrates the page.
> 
> [mhocko@suse.com: replace acct_isolated() with direct counting]
> ?

Why not. I just considered this patch more as a rework of the original
than an incremental fix. But whatever...
 
> Indeed much better than before. IIRC I've personally introduced one or two
> bugs involving acct_isolated() (lack of) usage :) Thanks.

Yeah, it was subtle as hell.

> > Fixes: bda807d44454 ("mm: migrate: support non-lru movable page migration")
> > Acked-by: Minchan Kim <minchan@kernel.org>
> > Signed-off-by: Ming Ling <ming.ling@spreadtrum.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!
-- 
Michal Hocko
SUSE Labs

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

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"ming.ling" <ming.ling@spreadtrum.com>,
	Minchan Kim <minchan@kernel.org>, Mel Gorman <mgorman@suse.de>,
	Joonsoo Kim <js1304@gmail.com>,
	linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm, compaction: fix NR_ISOLATED_* stats for pfn based migration
Date: Wed, 19 Oct 2016 12:36:16 +0200	[thread overview]
Message-ID: <20161019103616.GG7517@dhcp22.suse.cz> (raw)
In-Reply-To: <2e4d79f9-74e5-5085-4037-caa9c1cb43e4@suse.cz>

On Wed 19-10-16 11:39:36, Vlastimil Babka wrote:
> On 10/19/2016 10:02 AM, Michal Hocko wrote:
> > From: Ming Ling <ming.ling@spreadtrum.com>
> > 
> > Since bda807d44454 ("mm: migrate: support non-lru movable page
> > migration") isolate_migratepages_block) can isolate !PageLRU pages which
> > would acct_isolated account as NR_ISOLATED_*. Accounting these non-lru
> > pages NR_ISOLATED_{ANON,FILE} doesn't make any sense and it can misguide
> > heuristics based on those counters such as pgdat_reclaimable_pages resp.
> > too_many_isolated which would lead to unexpected stalls during the
> > direct reclaim without any good reason. Note that
> > __alloc_contig_migrate_range can isolate a lot of pages at once.
> > 
> > On mobile devices such as 512M ram android Phone, it may use a big zram
> > swap. In some cases zram(zsmalloc) uses too many non-lru but migratedable
> > pages, such as:
> > 
> >       MemTotal: 468148 kB
> >       Normal free:5620kB
> >       Free swap:4736kB
> >       Total swap:409596kB
> >       ZRAM: 164616kB(zsmalloc non-lru pages)
> >       active_anon:60700kB
> >       inactive_anon:60744kB
> >       active_file:34420kB
> >       inactive_file:37532kB
> > 
> > Fix this by only accounting lru pages to NR_ISOLATED_* in
> > isolate_migratepages_block right after they were isolated and we still
> > know they were on LRU. Drop acct_isolated because it is called after the
> > fact and we've lost that information. Batching per-cpu counter doesn't
> > make much improvement anyway. Also make sure that we uncharge only LRU
> > pages when putting them back on the LRU in putback_movable_pages resp.
> > when unmap_and_move migrates the page.
> 
> [mhocko@suse.com: replace acct_isolated() with direct counting]
> ?

Why not. I just considered this patch more as a rework of the original
than an incremental fix. But whatever...
 
> Indeed much better than before. IIRC I've personally introduced one or two
> bugs involving acct_isolated() (lack of) usage :) Thanks.

Yeah, it was subtle as hell.

> > Fixes: bda807d44454 ("mm: migrate: support non-lru movable page migration")
> > Acked-by: Minchan Kim <minchan@kernel.org>
> > Signed-off-by: Ming Ling <ming.ling@spreadtrum.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!
-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2016-10-19 10:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-19  8:02 [PATCH] mm, compaction: fix NR_ISOLATED_* stats for pfn based migration Michal Hocko
2016-10-19  8:02 ` Michal Hocko
2016-10-19  9:39 ` Vlastimil Babka
2016-10-19  9:39   ` Vlastimil Babka
2016-10-19 10:36   ` Michal Hocko [this message]
2016-10-19 10:36     ` Michal Hocko
2016-10-21  3:16 ` Andrew Morton
2016-10-21  3:16   ` Andrew Morton
2016-10-21  6:28   ` Michal Hocko
2016-10-21  6:28     ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161019103616.GG7517@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=ming.ling@spreadtrum.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.