| 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
| ||
|
Message-ID: <2e4d79f9-74e5-5085-4037-caa9c1cb43e4@suse.cz>
Date: Wed, 19 Oct 2016 11:39:36 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Michal Hocko <mhocko@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: "ming.ling" <ming.ling@...eadtrum.com>,
Minchan Kim <minchan@...nel.org>, Mel Gorman <mgorman@...e.de>,
Joonsoo Kim <js1304@...il.com>, linux-mm@...ck.org,
LKML <linux-kernel@...r.kernel.org>,
Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH] mm, compaction: fix NR_ISOLATED_* stats for pfn based
migration
On 10/19/2016 10:02 AM, Michal Hocko wrote:
> From: Ming Ling <ming.ling@...eadtrum.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@...e.com: replace acct_isolated() with direct counting]
?
Indeed much better than before. IIRC I've personally introduced one or two bugs
involving acct_isolated() (lack of) usage :) Thanks.
> Fixes: bda807d44454 ("mm: migrate: support non-lru movable page migration")
> Acked-by: Minchan Kim <minchan@...nel.org>
> Signed-off-by: Ming Ling <ming.ling@...eadtrum.com>
> Signed-off-by: Michal Hocko <mhocko@...e.com>
Acked-by: Vlastimil Babka <vbabka@...e.cz>
> ---
> mm/compaction.c | 25 +++----------------------
> mm/migrate.c | 15 +++++++++++----
> 2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 0409a4ad6ea1..70e6bec46dc2 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -634,22 +634,6 @@ isolate_freepages_range(struct compact_control *cc,
> return pfn;
> }
>
> -/* Update the number of anon and file isolated pages in the zone */
> -static void acct_isolated(struct zone *zone, struct compact_control *cc)
> -{
> - struct page *page;
> - unsigned int count[2] = { 0, };
> -
> - if (list_empty(&cc->migratepages))
> - return;
> -
> - list_for_each_entry(page, &cc->migratepages, lru)
> - count[!!page_is_file_cache(page)]++;
> -
> - mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_ANON, count[0]);
> - mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, count[1]);
> -}
> -
> /* Similar to reclaim, but different enough that they don't share logic */
> static bool too_many_isolated(struct zone *zone)
> {
> @@ -866,6 +850,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>
> /* Successfully isolated */
> del_page_from_lru_list(page, lruvec, page_lru(page));
> + inc_node_page_state(page,
> + NR_ISOLATED_ANON + page_is_file_cache(page));
>
> isolate_success:
> list_add(&page->lru, &cc->migratepages);
> @@ -902,7 +888,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> spin_unlock_irqrestore(zone_lru_lock(zone), flags);
> locked = false;
> }
> - acct_isolated(zone, cc);
> putback_movable_pages(&cc->migratepages);
> cc->nr_migratepages = 0;
> cc->last_migrated_pfn = 0;
> @@ -988,7 +973,6 @@ isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
> if (cc->nr_migratepages == COMPACT_CLUSTER_MAX)
> break;
> }
> - acct_isolated(cc->zone, cc);
>
> return pfn;
> }
> @@ -1258,10 +1242,8 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
> low_pfn = isolate_migratepages_block(cc, low_pfn,
> block_end_pfn, isolate_mode);
>
> - if (!low_pfn || cc->contended) {
> - acct_isolated(zone, cc);
> + if (!low_pfn || cc->contended)
> return ISOLATE_ABORT;
> - }
>
> /*
> * Either we isolated something and proceed with migration. Or
> @@ -1271,7 +1253,6 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
> break;
> }
>
> - acct_isolated(zone, cc);
> /* Record where migration scanner will be restarted. */
> cc->migrate_pfn = low_pfn;
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 99250aee1ac1..66ce6b490b13 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -168,8 +168,6 @@ void putback_movable_pages(struct list_head *l)
> continue;
> }
> list_del(&page->lru);
> - dec_node_page_state(page, NR_ISOLATED_ANON +
> - page_is_file_cache(page));
> /*
> * We isolated non-lru movable page so here we can use
> * __PageMovable because LRU page's mapping cannot have
> @@ -186,6 +184,8 @@ void putback_movable_pages(struct list_head *l)
> put_page(page);
> } else {
> putback_lru_page(page);
> + dec_node_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> }
> }
> }
> @@ -1121,8 +1121,15 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
> * restored.
> */
> list_del(&page->lru);
> - dec_node_page_state(page, NR_ISOLATED_ANON +
> - page_is_file_cache(page));
> +
> + /*
> + * Compaction can migrate also non-LRU pages which are
> + * not accounted to NR_ISOLATED_*. They can be recognized
> + * as __PageMovable
> + */
> + if (likely(!__PageMovable(page)))
> + dec_node_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> }
>
> /*
>
Powered by blists - more mailing lists