On Thu 13-10-16 14:39:09, ming.ling wrote:
> From: Ming Ling <ming.l...@spreadtrum.com>
>
> Non-lru pages don't belong to any lru, so counting them to
> NR_ISOLATED_ANON or NR_ISOLATED_FILE doesn't make any sense.
> It may misguide functions such as pgdat_reclaimable_pages and
> too_many_isolated.
That doesn't make much sense to me. I guess you wanted to say something
like
"
Accounting non-lru pages isolated for migration during pfn walk to
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. 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 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
I assume those zsmalloc pages are migrateable and that is the problem?
Please state that explicitly so that even people not familiar with
zsmalloc understand the motivation.
> More non-lru pages which used by zram for swap, it influences
> pgdat_reclaimable_pages and too_many_isolated more.
It would be good to mention what would be a visible effect of this.
"If the NR_ISOLATED_* is too large then the direct reclaim might get
throttled prematurely inducing longer allocation latencies without any
strong reason."
> This patch excludes isolated non-lru pages from NR_ISOLATED_ANON
> or NR_ISOLATED_FILE to ensure their counts are right.
But this patch doesn't do that. It just relies on __PageMovable. It is
true that all LRU pages should be movable (well except for
NR_UNEVICTABLE in certain configurations) but is it true that all
movable pages are on the LRU list?
Why don't you simply mimic what shrink_inactive_list does? Aka count the
number of isolated pages and then account them when appropriate?
> Signed-off-by: Ming ling <ming.l...@spreadtrum.com>
> ---
> mm/compaction.c | 6 ++++--
> mm/migrate.c | 9 +++++----
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 0409a4a..ed4c553 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -643,8 +643,10 @@ static void acct_isolated(struct zone *zone, struct
> compact_control *cc)
> if (list_empty(&cc->migratepages))
> return;
>
> - list_for_each_entry(page, &cc->migratepages, lru)
> - count[!!page_is_file_cache(page)]++;
> + list_for_each_entry(page, &cc->migratepages, lru) {
> + if (likely(!__PageMovable(page)))
> + 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]);
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 99250ae..abe48cc 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
> @@ -185,6 +183,8 @@ void putback_movable_pages(struct list_head *l)
> unlock_page(page);
> put_page(page);
> } else {
> + dec_node_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> putback_lru_page(page);
> }
> }
> @@ -1121,8 +1121,9 @@ 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));
> + if (likely(!__PageMovable(page)))
> + dec_node_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> }
>
> /*
> --
> 1.9.1
--
Michal Hocko
SUSE Labs
