| Message ID | 20190716212436.7137-1-yuzhao@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: replace list_move_tail() with add_page_to_lru_list_tail() | expand |
On Tue, 16 Jul 2019 15:24:36 -0600 Yu Zhao <yuzhao@google.com> 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); > }
Hi Andrew, On Wed, Jul 24, 2019 at 07:32:49PM -0700, Andrew Morton wrote: > On Tue, 16 Jul 2019 15:24:36 -0600 Yu Zhao <yuzhao@google.com> 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? It meant "normal deactivation from the pagevec full". The sentence is very odd to me, too. ;-( Let's remove the weird comment in this chance. Thanks.
diff --git a/mm/swap.c b/mm/swap.c index ae300397dfda..0226c5346560 100644 --- 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. */ - list_move_tail(&page->lru, &lruvec->lists[lru]); + add_page_to_lru_list_tail(page, lruvec, lru); __count_vm_event(PGROTATED); } @@ -844,17 +844,15 @@ void lru_add_page_tail(struct page *page, struct page *page_tail, get_page(page_tail); list_add_tail(&page_tail->lru, list); } else { - struct list_head *list_head; /* * Head page has not yet been counted, as an hpage, * so we must account for each subpage individually. * - * Use the standard add function to put page_tail on the list, - * but then correct its position so they all end up in order. + * Put page_tail on the list at the correct position + * so they all end up in order. */ - add_page_to_lru_list(page_tail, lruvec, page_lru(page_tail)); - list_head = page_tail->lru.prev; - list_move_tail(&page_tail->lru, list_head); + add_page_to_lru_list_tail(page_tail, lruvec, + page_lru(page_tail)); } if (!PageUnevictable(page))
This is a cleanup patch that replaces two historical uses of list_move_tail() with relatively recent add_page_to_lru_list_tail(). Signed-off-by: Yu Zhao <yuzhao@google.com> --- mm/swap.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)