Hi Hillf,
On Tue, Jun 04, 2019 at 12:20:47PM +0800, Hillf Danton wrote:
>
> Hi Minchan
>
> On Mon, 3 Jun 2019 13:37:27 +0800 Minchan Kim wrote:
> > @@ -1181,10 +1179,17 @@ static ICE_noinline int unmap_and_move(new_page_t
> > get_new_page,
> > return -ENOMEM;
> >
> > if (page_count(page) == 1) {
> > + bool is_lru = !__PageMovable(page);
> > +
> > /* page was freed from under us. So we are done. */
> > ClearPageActive(page);
> > ClearPageUnevictable(page);
> > - if (unlikely(__PageMovable(page))) {
> > + if (likely(is_lru))
> > + mod_node_page_state(page_pgdat(page),
> > + NR_ISOLATED_ANON +
> > + page_is_file_cache(page),
> > + hpage_nr_pages(page));
That should be -hpage_nr_pages(page). It's a bug.
> > + else {
> > lock_page(page);
> > if (!PageMovable(page))
> > __ClearPageIsolated(page);
>
> As this page will go down the path only through the MIGRATEPAGE_SUCCESS
> branches,
> with no putback ahead, the current code is, I think, doing right things for
> this
> work to keep isolated stat balanced.
I guess that's the one you pointed out. Right?
Thanks for the review!
>
> > @@ -1210,15 +1215,6 @@ static ICE_noinline int unmap_and_move(new_page_t
> > get_new_page,
> > * restored.
> > */
> > list_del(&page->lru);
> > -
> > - /*
> > - * Compaction can migrate also non-LRU pages which are
> > - * not accounted to NR_ISOLATED_*. They can be recognized
> > - * as __PageMovable
> > - */
> > - if (likely(!__PageMovable(page)))
> > - mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
> > - page_is_file_cache(page),
> > -hpage_nr_pages(page));
> > }
> >
>
> BR
> Hillf
>
