The patch titled Subject: mm/vmscan: __isolate_lru_page_prepare() cleanup has been added to the -mm tree. Its filename is mm-vmscan-__isolate_lru_page_prepare-clean-up.patch This patch should soon appear at https://ozlabs.org/~akpm/mmots/broken-out/mm-vmscan-__isolate_lru_page_prepare-clean-up.patch and later at https://ozlabs.org/~akpm/mmotm/broken-out/mm-vmscan-__isolate_lru_page_prepare-clean-up.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** The -mm tree is included into linux-next and is updated there every 3-4 working days ------------------------------------------------------ From: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx> Subject: mm/vmscan: __isolate_lru_page_prepare() cleanup The function just returns 2 results, so using a 'switch' to deal with its result is unnecessary. Also simplify it to a bool func as Vlastimil suggested. Also remove 'goto' by reusing list_move(), and take Matthew Wilcox's suggestion to update comments in function. Link: https://lkml.kernel.org/r/728874d7-2d93-4049-68c1-dcc3b2d52ccd@xxxxxxxxxxxxxxxxx Signed-off-by: Alex Shi <alex.shi@xxxxxxxxxxxxxxxxx> Reviewed-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> Cc: Hugh Dickins <hughd@xxxxxxxxxx> Cc: Yu Zhao <yuzhao@xxxxxxxxxx> Cc: Vlastimil Babka <vbabka@xxxxxxx> Cc: Michal Hocko <mhocko@xxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- include/linux/swap.h | 2 - mm/compaction.c | 2 - mm/vmscan.c | 68 ++++++++++++++++++----------------------- 3 files changed, 33 insertions(+), 39 deletions(-) --- a/include/linux/swap.h~mm-vmscan-__isolate_lru_page_prepare-clean-up +++ a/include/linux/swap.h @@ -356,7 +356,7 @@ extern void lru_cache_add_inactive_or_un extern unsigned long zone_reclaimable_pages(struct zone *zone); extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order, gfp_t gfp_mask, nodemask_t *mask); -extern int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode); +extern bool __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode); extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg, unsigned long nr_pages, gfp_t gfp_mask, --- a/mm/compaction.c~mm-vmscan-__isolate_lru_page_prepare-clean-up +++ a/mm/compaction.c @@ -988,7 +988,7 @@ isolate_migratepages_block(struct compac if (unlikely(!get_page_unless_zero(page))) goto isolate_fail; - if (__isolate_lru_page_prepare(page, isolate_mode) != 0) + if (!__isolate_lru_page_prepare(page, isolate_mode)) goto isolate_fail_put; /* Try isolate the page */ --- a/mm/vmscan.c~mm-vmscan-__isolate_lru_page_prepare-clean-up +++ a/mm/vmscan.c @@ -1536,19 +1536,17 @@ unsigned int reclaim_clean_pages_from_li * page: page to consider * mode: one of the LRU isolation modes defined above * - * returns 0 on success, -ve errno on failure. + * returns true on success, false on failure. */ -int __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode) +bool __isolate_lru_page_prepare(struct page *page, isolate_mode_t mode) { - int ret = -EBUSY; - /* Only take pages on the LRU. */ if (!PageLRU(page)) - return ret; + return false; /* Compaction should not handle unevictable pages but CMA can do so */ if (PageUnevictable(page) && !(mode & ISOLATE_UNEVICTABLE)) - return ret; + return false; /* * To minimise LRU disruption, the caller can indicate that it only @@ -1561,7 +1559,7 @@ int __isolate_lru_page_prepare(struct pa if (mode & ISOLATE_ASYNC_MIGRATE) { /* All the caller can do on PageWriteback is block */ if (PageWriteback(page)) - return ret; + return false; if (PageDirty(page)) { struct address_space *mapping; @@ -1577,20 +1575,20 @@ int __isolate_lru_page_prepare(struct pa * from the page cache. */ if (!trylock_page(page)) - return ret; + return false; mapping = page_mapping(page); migrate_dirty = !mapping || mapping->a_ops->migratepage; unlock_page(page); if (!migrate_dirty) - return ret; + return false; } } if ((mode & ISOLATE_UNMAPPED) && page_mapped(page)) - return ret; + return false; - return 0; + return true; } /* @@ -1674,35 +1672,31 @@ static unsigned long isolate_lru_pages(u * only when the page is being freed somewhere else. */ scan += nr_pages; - switch (__isolate_lru_page_prepare(page, mode)) { - case 0: - /* - * Be careful not to clear PageLRU until after we're - * sure the page is not being freed elsewhere -- the - * page release code relies on it. - */ - if (unlikely(!get_page_unless_zero(page))) - goto busy; + if (!__isolate_lru_page_prepare(page, mode)) { + /* It is being freed elsewhere */ + list_move(&page->lru, src); + continue; + } + /* + * Be careful not to clear PageLRU until after we're + * sure the page is not being freed elsewhere -- the + * page release code relies on it. + */ + if (unlikely(!get_page_unless_zero(page))) { + list_move(&page->lru, src); + continue; + } - if (!TestClearPageLRU(page)) { - /* - * This page may in other isolation path, - * but we still hold lru_lock. - */ - put_page(page); - goto busy; - } - - nr_taken += nr_pages; - nr_zone_taken[page_zonenum(page)] += nr_pages; - list_move(&page->lru, dst); - break; - - default: -busy: - /* else it is being freed elsewhere */ + if (!TestClearPageLRU(page)) { + /* Another thread is already isolating this page */ + put_page(page); list_move(&page->lru, src); + continue; } + + nr_taken += nr_pages; + nr_zone_taken[page_zonenum(page)] += nr_pages; + list_move(&page->lru, dst); } /* _ Patches currently in -mm which might be from alex.shi@xxxxxxxxxxxxxxxxx are mm-filemap-add-static-for-function-__add_to_page_cache_locked.patch fs-ntfs-remove-unused-varibles.patch fs-ntfs-remove-unused-varible-attr_len.patch mm-truncate-add-parameter-explanation-for-invalidate_mapping_pagevec.patch mm-memcg-update-page-struct-member-in-comments.patch mm-thp-move-lru_add_page_tail-func-to-huge_memoryc.patch mm-thp-use-head-for-head-page-in-lru_add_page_tail.patch mm-thp-simplify-lru_add_page_tail.patch mm-thp-narrow-lru-locking.patch mm-vmscan-remove-unnecessary-lruvec-adding.patch mm-rmap-stop-store-reordering-issue-on-page-mapping.patch mm-rmap-stop-store-reordering-issue-on-page-mapping-fix.patch mm-memcg-add-debug-checking-in-lock_page_memcg.patch mm-swapc-fold-vm-event-pgrotated-into-pagevec_move_tail_fn.patch mm-lru-move-lock-into-lru_note_cost.patch mm-vmscan-remove-lruvec-reget-in-move_pages_to_lru.patch mm-mlock-remove-lru_lock-on-testclearpagemlocked.patch mm-mlock-remove-__munlock_isolate_lru_page.patch mm-lru-introduce-testclearpagelru.patch mm-compaction-do-page-isolation-first-in-compaction.patch mm-swapc-serialize-memcg-changes-in-pagevec_lru_move_fn.patch mm-lru-replace-pgdat-lru_lock-with-lruvec-lock.patch mm-lru-replace-pgdat-lru_lock-with-lruvec-lock-fix.patch mm-lru-replace-pgdat-lru_lock-with-lruvec-lock-fix-2.patch mm-lru-introduce-the-relock_page_lruvec-function-fix.patch mm-memcg-remove-incorrect-comments.patch mm-mapping_dirty_helpers-enhance-the-kernel-doc-markups.patch mm-add-colon-to-fix-kernel-doc-markups-error-for-check_pte.patch mm-vmalloc-add-align-parameter-explanation-for-pvm_determine_end_from_reverse.patch docs-vm-remove-unused-3-items-explanation-for-proc-vmstat.patch khugepaged-add-couples-parameter-explanation-for-kernel-doc-markup.patch mm-vmscan-__isolate_lru_page_prepare-clean-up.patch gcov-fix-kernel-doc-markup-issue.patch mm-swapc-reduce-lock-contention-in-lru_cache_add.patch mm-memcg-bail-early-from-swap-accounting-if-memcg-disabled.patch mm-memcg-warning-on-memcg-after-readahead-page-charged.patch mm-memcontrol-rewrite-mem_cgroup_page_lruvec-fix.patch