Hello,
On 五, 9月 30, 2016 at 11:37:37上午 +0900, Minchan Kim wrote:
> Hello,
>
> On Wed, Sep 28, 2016 at 05:31:03PM +0800, ming.ling wrote:
> > Non-lru pages don't belong to any lru, so accounting 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.
>
> I agree this part. It would be happier if you give any story you suffered
> from. Although you don't have, it's okay because you are correcting
> clearly wrong part. Thanks. :)
>
I haven't suffered from any actual problem yet. It is very hard to make a
special case making reclaim artificially throttled (hung) because of too
many non LRU pages being isolated. But since i do some develpments on low ram
Android devices（512M...), i am very sensitive on memory footprint analysis.
Incorrect counting of non-lru pages makes me confused on it.
And patch "mm, vmscan: consider isolated pages in zone_reclaimable_pages
(9f6c399d)" which had been committed by Michal Hocko adds isolated pages in
zone_reclaimable_pages. So i think it is worth to ensure their counts are right.
> >
> > This patch adds NR_ISOLATED_NONLRU to vmstat and moves isolated non-lru
> > pages from NR_ISOLATED_ANON or NR_ISOLATED_FILE to NR_ISOLATED_NONLRU.
> > And with non-lru pages in vmstat, it helps to optimize algorithm of
> > function too_many_isolated oneday.
>
> Need more justfication to add new vmstat because once we add it, it's
> really hard to change/remove it(i.e., maintainace trobule) so I want
> to add it when it really would be helpful sometime, not now.
>
> Could you resend the patch without part adding new vmstat?
>
> Thanks.
>
No problem, let's add it when it really would be helpful sometime.
I will resend the patch without part adding new vmstat later.
Thank you very much.
> >
> > Signed-off-by: ming.ling <ming.ling@xxxxxxxxxxxxxx>
> > ---
> > include/linux/mmzone.h | 1 +
> > mm/compaction.c | 12 +++++++++---
> > mm/migrate.c | 14 ++++++++++----
> > 3 files changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index 7f2ae99..dc0adba 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -169,6 +169,7 @@ enum node_stat_item {
> > NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */
> > NR_DIRTIED, /* page dirtyings since bootup */
> > NR_WRITTEN, /* page writings since bootup */
> > + NR_ISOLATED_NONLRU, /* Temporary isolated pages from non-lru */
> > NR_VM_NODE_STAT_ITEMS
> > };
> >
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 9affb29..8da1dca 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -638,16 +638,21 @@ isolate_freepages_range(struct compact_control *cc,
> > static void acct_isolated(struct zone *zone, struct compact_control *cc)
> > {
> > struct page *page;
> > - unsigned int count[2] = { 0, };
> > + unsigned int count[3] = { 0, };
> >
> > 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 (PageLRU(page))
> > + count[!!page_is_file_cache(page)]++;
> > + else
> > + count[2]++;
> > + }
> >
> > mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_ANON, count[0]);
> > mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, count[1]);
> > + mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_NONLRU, count[2]);
> > }
> >
> > /* Similar to reclaim, but different enough that they don't share logic */
> > @@ -659,6 +664,7 @@ static bool too_many_isolated(struct zone *zone)
> > node_page_state(zone->zone_pgdat, NR_INACTIVE_ANON);
> > active = node_page_state(zone->zone_pgdat, NR_ACTIVE_FILE) +
> > node_page_state(zone->zone_pgdat, NR_ACTIVE_ANON);
> > + /* Is it necessary to add NR_ISOLATED_NONLRU?? */
> > isolated = node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE) +
> > node_page_state(zone->zone_pgdat, NR_ISOLATED_ANON);
> >
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index f7ee04a..cd5abb2 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -168,8 +168,11 @@ 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));
> > + if (PageLRU(page))
> > + dec_node_page_state(page, NR_ISOLATED_ANON +
> > + page_is_file_cache(page));
> > + else
> > + dec_node_page_state(page, NR_ISOLATED_NONLRU);
> > /*
> > * We isolated non-lru movable page so here we can use
> > * __PageMovable because LRU page's mapping cannot have
> > @@ -1121,8 +1124,11 @@ out:
> > * restored.
> > */
> > list_del(&page->lru);
> > - dec_node_page_state(page, NR_ISOLATED_ANON +
> > - page_is_file_cache(page));
> > + if (PageLRU(page))
> > + dec_node_page_state(page, NR_ISOLATED_ANON +
> > + page_is_file_cache(page));
> > + else
> > + dec_node_page_state(page, NR_ISOLATED_NONLRU);
> > }
> >
> > /*
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>