Re: [PATCH] vmscan: reclaim_clean_pages_from_list() must count mlocked pages

[Minchan Kim]

Hello,

On Tue, Aug 04, 2015 at 12:46:57AM +0900, Jaewon Kim wrote:
> 
> 
> On 2015ë 08ì 04ì 00:33, Minchan Kim wrote:
> > On Mon, Aug 03, 2015 at 11:55:46PM +0900, Jaewon Kim wrote:
> >>
> >>
> >> On 2015ë 08ì 03ì 21:27, Minchan Kim wrote:
> >>> Hello,
> >>>
> >>> On Mon, Aug 03, 2015 at 07:18:27PM +0900, Jaewon Kim wrote:
> >>>> reclaim_clean_pages_from_list() decreases NR_ISOLATED_FILE by returned
> >>>> value from shrink_page_list(). But mlocked pages in the isolated
> >>>> clean_pages page list would be removed from the list but not counted as
> >>>> nr_reclaimed. Fix this miscounting by returning the number of mlocked
> >>>> pages and count it.
> >>>
> >>> If there are pages not able to reclaim, VM try to migrate it and
> >>> have to handle the stat in migrate_pages.
> >>> If migrate_pages fails again, putback-fiends should handle it.
> >>>
> >>> Is there anyting I am missing now?
> >>>
> >>> Thanks.
> >>>
> >> Hello
> >>
> >> Only pages in cc->migratepages will be handled by migrate_pages or
> >> putback_movable_pages, and NR_ISOLATED_FILE will be counted properly.
> >> However mlocked pages will not be put back into cc->migratepages,
> >> and also not be counted in NR_ISOLATED_FILE because putback_lru_page
> >> in shrink_page_list does not increase NR_ISOLATED_FILE.
> >> The current reclaim_clean_pages_from_list assumes that shrink_page_list
> >> returns number of pages removed from the candidate list.
> >>
> >> i.e)
> >> isolate_migratepages_range    : NR_ISOLATED_FILE += 10
> >> reclaim_clean_pages_from_list : NR_ISOLATED_FILE -= 5 (1 mlocked page)
> >> migrate_pages                 : NR_ISOLATED_FILE -=4
> >> => NR_ISOLATED_FILE increased by 1
> > 
> > Thanks for the clarity.
> > 
> > I think the problem is shrink_page_list is awkard. It put back to
> > unevictable pages instantly instead of passing it to caller while
> > it relies on caller for non-reclaimed-non-unevictable page's putback.
> > 
> > I think we can make it consistent so that shrink_page_list could
> > return non-reclaimed pages via page_list and caller can handle it.
> > As a bonus, it could try to migrate mlocked pages without retrial.
> > 
> >>
> >> Thank you.
> 
> To make clear do you mean changing shrink_page_list like this rather than
> previous my suggestion?
> 
> @@ -1157,7 +1157,7 @@ cull_mlocked:
>                 if (PageSwapCache(page))
>                         try_to_free_swap(page);
>                 unlock_page(page);
> -               putback_lru_page(page);
> +               list_add(&page->lru, &ret_pages);
>                 continue;

Yes. That's what I said.

Thanks.

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/