Re: [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE list
From: Johannes Weiner
Date: Thu Feb 23 2017 - 11:04:31 EST
Hi Shaohua,
On Wed, Feb 22, 2017 at 10:50:41AM -0800, Shaohua Li wrote:
> @@ -268,6 +268,12 @@ static void __activate_page(struct page *page, struct lruvec *lruvec,
> int lru = page_lru_base_type(page);
>
> del_page_from_lru_list(page, lruvec, lru);
> + if (PageAnon(page) && !PageSwapBacked(page)) {
> + SetPageSwapBacked(page);
> + /* charge to anon scanned/rotated reclaim_stat */
> + file = 0;
> + lru = LRU_INACTIVE_ANON;
> + }
As per my previous feedback, please remove this. Write-after-free will
be caught and handled in the reclaimer, read-after-free is a bug that
really doesn't require optimizing page aging for. And we definitely
shouldn't declare invalid data suddenly valid because it's being read.
> @@ -561,20 +567,26 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
> }
>
>
> -static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
> +static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
> void *arg)
> {
> - if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
> - int file = page_is_file_cache(page);
> - int lru = page_lru_base_type(page);
> + if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) &&
> + !PageUnevictable(page)) {
> + bool active = PageActive(page);
>
> - del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
> + del_page_from_lru_list(page, lruvec, LRU_INACTIVE_ANON + active);
> ClearPageActive(page);
> ClearPageReferenced(page);
> - add_page_to_lru_list(page, lruvec, lru);
> + /*
> + * lazyfree pages are clean anonymous pages. They have
> + * SwapBacked flag cleared to destinguish normal anonymous
> + * pages
distinguish
Otherwise, looks great to me. Thanks!