Messages in this thread |  | | Date | Sun, 22 Nov 2020 12:35:52 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH next] mm/vmscan: __isolate_lru_page_prepare clean up |
| |
On Sun, Nov 22, 2020 at 08:00:19PM +0800, Alex Shi wrote: > mm/compaction.c | 2 +- > mm/vmscan.c | 69 +++++++++++++++++++++++-------------------------- > 2 files changed, 34 insertions(+), 37 deletions(-)
How is it possible you're changing the signature of a function without touching a header file? Surely __isolate_lru_page_prepare() must be declared in mm/internal.h ?
> +++ b/mm/vmscan.c > @@ -1536,19 +1536,17 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, > * page: page to consider > * mode: one of the LRU isolation modes defined above > * > - * returns 0 on success, -ve errno on failure. > + * returns ture on success, false on failure.
"true".
> @@ -1674,35 +1672,34 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, > * only when the page is being freed somewhere else. > */ > scan += nr_pages; > - switch (__isolate_lru_page_prepare(page, mode)) { > - case 0: > + if (!__isolate_lru_page_prepare(page, mode)) { > + /* else it is being freed elsewhere */
I don't think the word "else" helps here. Just /* It is being freed elsewhere */
> + if (!TestClearPageLRU(page)) { > /* > + * This page may in other isolation path, > + * but we still hold lru_lock. > */
I don't think this comment helps me understand what's going on here. Maybe:
/* Another thread is already isolating this page */
> + put_page(page); > list_move(&page->lru, src); > + continue; > }
|  |