On Tue, 16 Jul 2019 15:24:36 -0600 Yu Zhao <yuzhao@xxxxxxxxxx> wrote:
> This is a cleanup patch that replaces two historical uses of
> list_move_tail() with relatively recent add_page_to_lru_list_tail().
>
Looks OK to me.
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -515,7 +515,6 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
> del_page_from_lru_list(page, lruvec, lru + active);
> ClearPageActive(page);
> ClearPageReferenced(page);
> - add_page_to_lru_list(page, lruvec, lru);
>
> if (PageWriteback(page) || PageDirty(page)) {
> /*
> @@ -523,13 +522,14 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
> * It can make readahead confusing. But race window
> * is _really_ small and it's non-critical problem.
> */
> + add_page_to_lru_list(page, lruvec, lru);
> SetPageReclaim(page);
> } else {
> /*
> * The page's writeback ends up during pagevec
> * We moves tha page into tail of inactive.
> */
That comment is really hard to follow. Minchan, can you please explain
the first sentence?
The second sentence can simply be removed.
> - list_move_tail(&page->lru, &lruvec->lists[lru]);
> + add_page_to_lru_list_tail(page, lruvec, lru);
> __count_vm_event(PGROTATED);
> }
