[PATCH v2 0/4] Cleanups for vmscan

[PATCH v2 0/4] Cleanups for vmscan

[Miaohe Lin]

Hi all,
This series contains cleanups to remove unneeded return value, misleading
setting and so on. Also this remove the PageDirty check after MADV_FREE
pages are page_ref_freezed. More details can be found in the respective
changelogs. Thanks!

v1->v2:
  collect Acked-by tag
  remove patch "mm/vmscan: fix misleading comment in isolate_lru_pages()"
  remove PageDirty check instead of setting SwapBacked in shrink_page_list

Miaohe Lin (4):
  mm/vmscan: remove the PageDirty check after MADV_FREE pages are
    page_ref_freezed
  mm/vmscan: remove misleading setting to sc->priority
  mm/vmscan: remove unneeded return value of kswapd_run()
  mm/vmscan: add 'else' to remove check_pending label

 include/linux/swap.h |  2 +-
 mm/vmscan.c          | 36 +++++++++++++++---------------------
 2 files changed, 16 insertions(+), 22 deletions(-)

-- 
2.23.0

[PATCH v2 1/4] mm/vmscan: remove the PageDirty check after MADV_FREE pages are page_ref_freezed

[Miaohe Lin]

If the MADV_FREE pages are redirtied before they could be reclaimed,
put the pages back to anonymous LRU list by setting SwapBacked flag
and the pages will be reclaimed in normal swapout way. But as Yu Zhao
pointed out, "The page has only one reference left, which is from the
isolation. After the caller puts the page back on lru and drops the
reference, the page will be freed anyway. It doesn't matter which lru
it goes." So we don't bother checking PageDirty here.

[Yu Zhao's comment is also quoted in the code.]

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/vmscan.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index a7602f71ec04..92a515e82b1b 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1627,11 +1627,14 @@ static unsigned int shrink_page_list(struct list_head *page_list,
 			/* follow __remove_mapping for reference */
 			if (!page_ref_freeze(page, 1))
 				goto keep_locked;
-			if (PageDirty(page)) {
-				page_ref_unfreeze(page, 1);
-				goto keep_locked;
-			}
-
+			/*
+			 * The page has only one reference left, which is
+			 * from the isolation. After the caller puts the
+			 * page back on lru and drops the reference, the
+			 * page will be freed anyway. It doesn't matter
+			 * which lru it goes. So we don't bother checking
+			 * PageDirty here.
+			 */
 			count_vm_event(PGLAZYFREED);
 			count_memcg_page_event(page, PGLAZYFREED);
 		} else if (!mapping || !__remove_mapping(mapping, page, true,
-- 
2.23.0

[PATCH v2 2/4] mm/vmscan: remove misleading setting to sc->priority

[Miaohe Lin]

The priority field of sc is used to control how many pages we should scan
at once while we always traverse the list to shrink the pages in these
functions. So these settings are unneeded and misleading.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/vmscan.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 92a515e82b1b..a55266685eb5 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1704,7 +1704,6 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone,
 {
 	struct scan_control sc = {
 		.gfp_mask = GFP_KERNEL,
-		.priority = DEF_PRIORITY,
 		.may_unmap = 1,
 	};
 	struct reclaim_stat stat;
@@ -2329,7 +2328,6 @@ unsigned long reclaim_pages(struct list_head *page_list)
 	unsigned int noreclaim_flag;
 	struct scan_control sc = {
 		.gfp_mask = GFP_KERNEL,
-		.priority = DEF_PRIORITY,
 		.may_writepage = 1,
 		.may_unmap = 1,
 		.may_swap = 1,
-- 
2.23.0

[PATCH v2 3/4] mm/vmscan: remove unneeded return value of kswapd_run()

[Miaohe Lin]

The return value of kswapd_run() is unused now. Clean it up.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/swap.h | 2 +-
 mm/vmscan.c          | 7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 6f5a43251593..717e6e500929 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -408,7 +408,7 @@ static inline bool node_reclaim_enabled(void)
 
 extern void check_move_unevictable_pages(struct pagevec *pvec);
 
-extern int kswapd_run(int nid);
+extern void kswapd_run(int nid);
 extern void kswapd_stop(int nid);
 
 #ifdef CONFIG_SWAP
diff --git a/mm/vmscan.c b/mm/vmscan.c
index a55266685eb5..260d900db20d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4286,23 +4286,20 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim)
  * This kswapd start function will be called by init and node-hot-add.
  * On node-hot-add, kswapd will moved to proper cpus if cpus are hot-added.
  */
-int kswapd_run(int nid)
+void kswapd_run(int nid)
 {
 	pg_data_t *pgdat = NODE_DATA(nid);
-	int ret = 0;
 
 	if (pgdat->kswapd)
-		return 0;
+		return;
 
 	pgdat->kswapd = kthread_run(kswapd, pgdat, "kswapd%d", nid);
 	if (IS_ERR(pgdat->kswapd)) {
 		/* failure at boot is fatal */
 		BUG_ON(system_state < SYSTEM_RUNNING);
 		pr_err("Failed to start kswapd on node %d\n", nid);
-		ret = PTR_ERR(pgdat->kswapd);
 		pgdat->kswapd = NULL;
 	}
-	return ret;
 }
 
 /*
-- 
2.23.0

[PATCH v2 4/4] mm/vmscan: add 'else' to remove check_pending label

[Miaohe Lin]

We could add 'else' to remove the somewhat odd check_pending label to
make code core succinct.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 mm/vmscan.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 260d900db20d..d226f7f1f2c4 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3430,18 +3430,14 @@ static bool throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,
 	 * blocked waiting on the same lock. Instead, throttle for up to a
 	 * second before continuing.
 	 */
-	if (!(gfp_mask & __GFP_FS)) {
+	if (!(gfp_mask & __GFP_FS))
 		wait_event_interruptible_timeout(pgdat->pfmemalloc_wait,
 			allow_direct_reclaim(pgdat), HZ);
+	else
+		/* Throttle until kswapd wakes the process */
+		wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
+			allow_direct_reclaim(pgdat));
 
-		goto check_pending;
-	}
-
-	/* Throttle until kswapd wakes the process */
-	wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
-		allow_direct_reclaim(pgdat));
-
-check_pending:
 	if (fatal_signal_pending(current))
 		return true;
 
-- 
2.23.0