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;
> }
