[PATCH v3 0/6] A few cleanup and fixup patches for vmscan

[PATCH v3 0/6] A few cleanup and fixup patches for vmscan

[Miaohe Lin]

Hi everyone,
This series contains a few patches to remove obsolete comment, introduce
helper to remove duplicated code and so no. Also we take all base pages
of THP into account in rare race condition. More details can be found in
the respective changelogs. Thanks!

---
v3:
  patch 1/9: collect Reviewed-by tag and add a period in the comment.
  patch 2/9: dropped
  patch 3/9: use folio_putback_lru()
  patch 4/9: dropped
  patch 5/9: collect Reviewed-by tag
  patch 7/9: splitted out to make it easier to move forward
v2:
  patch 1/9: drop code change and add a comment about MADV_FREE
  patch 2/9: simplify the code further and change to goto keep_locked
  patch 3/9: use folio, remove unneeded inline and break craze long lines
  patch 5/9: activate swap-backed executable folios after first usage too
  patch 9/9: new cleanup patch splitted from 5/9
  Many thanks Huang Ying, Matthew Wilcox, Christoph Hellwig, Muchun Song
  for review!
---

Miaohe Lin (6):
  mm/vmscan: add a comment about MADV_FREE pages check in
    folio_check_dirty_writeback
  mm/vmscan: introduce helper function reclaim_page_list()
  mm/vmscan: activate swap-backed executable folios after first usage
  mm/vmscan: take all base pages of THP into account when race with
    speculative reference
  mm/vmscan: remove obsolete comment in kswapd_run
  mm/vmscan: use helper folio_is_file_lru()

 mm/vmscan.c | 64 +++++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

-- 
2.23.0

[PATCH v3 1/6] mm/vmscan: add a comment about MADV_FREE pages check in folio_check_dirty_writeback

[Miaohe Lin]

The MADV_FREE pages check in folio_check_dirty_writeback is a bit hard to
follow. Add a comment to make the code clear.

Suggested-by: Huang, Ying <ying.huang@intel.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
---
 mm/vmscan.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 99a572f01cb4..913eb565140c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1438,7 +1438,10 @@ static void folio_check_dirty_writeback(struct folio *folio,
 
 	/*
 	 * Anonymous pages are not handled by flushers and must be written
-	 * from reclaim context. Do not stall reclaim based on them
+	 * from reclaim context. Do not stall reclaim based on them.
+	 * MADV_FREE anonymous pages are put into inactive file list too.
+	 * They could be mistakenly treated as file lru. So further anon
+	 * test is needed.
 	 */
 	if (!folio_is_file_lru(folio) ||
 	    (folio_test_anon(folio) && !folio_test_swapbacked(folio))) {
-- 
2.23.0

[PATCH v3 2/6] mm/vmscan: introduce helper function reclaim_page_list()

[Miaohe Lin]

Introduce helper function reclaim_page_list() to eliminate the duplicated
code of doing shrink_page_list() and putback_lru_page. Also We can separate
node reclaim from node page list operation this way. No functional change
intended.

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

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 913eb565140c..7dcc8cf4526f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2539,14 +2539,12 @@ static void shrink_active_list(unsigned long nr_to_scan,
 			nr_deactivate, nr_rotated, sc->priority, file);
 }
 
-unsigned long reclaim_pages(struct list_head *page_list)
+static unsigned int reclaim_page_list(struct list_head *page_list,
+				      struct pglist_data *pgdat)
 {
-	int nid = NUMA_NO_NODE;
-	unsigned int nr_reclaimed = 0;
-	LIST_HEAD(node_page_list);
 	struct reclaim_stat dummy_stat;
-	struct page *page;
-	unsigned int noreclaim_flag;
+	unsigned int nr_reclaimed;
+	struct folio *folio;
 	struct scan_control sc = {
 		.gfp_mask = GFP_KERNEL,
 		.may_writepage = 1,
@@ -2555,6 +2553,24 @@ unsigned long reclaim_pages(struct list_head *page_list)
 		.no_demotion = 1,
 	};
 
+	nr_reclaimed = shrink_page_list(page_list, pgdat, &sc, &dummy_stat, false);
+	while (!list_empty(page_list)) {
+		folio = lru_to_folio(page_list);
+		list_del(&folio->lru);
+		folio_putback_lru(folio);
+	}
+
+	return nr_reclaimed;
+}
+
+unsigned long reclaim_pages(struct list_head *page_list)
+{
+	int nid = NUMA_NO_NODE;
+	unsigned int nr_reclaimed = 0;
+	LIST_HEAD(node_page_list);
+	struct page *page;
+	unsigned int noreclaim_flag;
+
 	noreclaim_flag = memalloc_noreclaim_save();
 
 	while (!list_empty(page_list)) {
@@ -2570,28 +2586,12 @@ unsigned long reclaim_pages(struct list_head *page_list)
 			continue;
 		}
 
-		nr_reclaimed += shrink_page_list(&node_page_list,
-						NODE_DATA(nid),
-						&sc, &dummy_stat, false);
-		while (!list_empty(&node_page_list)) {
-			page = lru_to_page(&node_page_list);
-			list_del(&page->lru);
-			putback_lru_page(page);
-		}
-
+		nr_reclaimed += reclaim_page_list(&node_page_list, NODE_DATA(nid));
 		nid = NUMA_NO_NODE;
 	}
 
-	if (!list_empty(&node_page_list)) {
-		nr_reclaimed += shrink_page_list(&node_page_list,
-						NODE_DATA(nid),
-						&sc, &dummy_stat, false);
-		while (!list_empty(&node_page_list)) {
-			page = lru_to_page(&node_page_list);
-			list_del(&page->lru);
-			putback_lru_page(page);
-		}
-	}
+	if (!list_empty(&node_page_list))
+		nr_reclaimed += reclaim_page_list(&node_page_list, NODE_DATA(nid));
 
 	memalloc_noreclaim_restore(noreclaim_flag);
 
-- 
2.23.0

[PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[Miaohe Lin]

We should activate swap-backed executable folios (e.g. tmpfs) after first
usage so that executable code gets yet better chance to stay in memory.

Suggested-by: Huang, Ying <ying.huang@intel.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Huang, Ying <ying.huang@intel.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 mm/vmscan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7dcc8cf4526f..82e6fa83c472 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1415,9 +1415,9 @@ static enum page_references folio_check_references(struct folio *folio,
 			return PAGEREF_ACTIVATE;
 
 		/*
-		 * Activate file-backed executable folios after first usage.
+		 * Activate executable folios after first usage.
 		 */
-		if ((vm_flags & VM_EXEC) && !folio_test_swapbacked(folio))
+		if (vm_flags & VM_EXEC)
 			return PAGEREF_ACTIVATE;
 
 		return PAGEREF_KEEP;
-- 
2.23.0

[PATCH v3 4/6] mm/vmscan: take all base pages of THP into account when race with speculative reference

[Miaohe Lin]

If the page has buffers, shrink_page_list will try to free the buffer
mappings associated with the page and try to free the page as well.
In the rare race with speculative reference, the page will be freed
shortly by speculative reference. But nr_reclaimed is not incremented
correctly when we come across the THP. We need to account all the base
pages in this case.

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

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 82e6fa83c472..10ba16fdd748 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1887,7 +1887,7 @@ static unsigned int shrink_page_list(struct list_head *page_list,
 					 * increment nr_reclaimed here (and
 					 * leave it off the LRU).
 					 */
-					nr_reclaimed++;
+					nr_reclaimed += nr_pages;
 					continue;
 				}
 			}
-- 
2.23.0

[PATCH v3 5/6] mm/vmscan: remove obsolete comment in kswapd_run

[Miaohe Lin]

Since commit 6b700b5b3c59 ("mm/vmscan.c: remove cpu online notification
for now"), cpu online notification is removed. So kswapd won't move to
proper cpus if cpus are hot-added. Remove this obsolete comment.

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

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 10ba16fdd748..db3500bd5c7d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4572,7 +4572,6 @@ 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.
  */
 void kswapd_run(int nid)
 {
-- 
2.23.0

[PATCH v3 6/6] mm/vmscan: use helper folio_is_file_lru()

[Miaohe Lin]

Use helper folio_is_file_lru() to check whether folio is file lru. Minor
readability improvement.

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

diff --git a/mm/vmscan.c b/mm/vmscan.c
index db3500bd5c7d..a2752e8fc879 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1424,7 +1424,7 @@ static enum page_references folio_check_references(struct folio *folio,
 	}
 
 	/* Reclaim if clean, defer dirty folios to writeback */
-	if (referenced_folio && !folio_test_swapbacked(folio))
+	if (referenced_folio && folio_is_file_lru(folio))
 		return PAGEREF_RECLAIM_CLEAN;
 
 	return PAGEREF_RECLAIM;
-- 
2.23.0

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[Andrew Morton]

On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> We should activate swap-backed executable folios (e.g. tmpfs) after first
> usage so that executable code gets yet better chance to stay in memory.

Does this not erase the intended effect of Joonsoo's b518154e59a
("mm/vmscan: protect the workingset on anonymous LRU")?

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[ying.huang]

On Mon, 2022-04-25 at 14:07 -0700, Andrew Morton wrote:
> On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
> > We should activate swap-backed executable folios (e.g. tmpfs) after first
> > usage so that executable code gets yet better chance to stay in memory.
> 
> Does this not erase the intended effect of Joonsoo's b518154e59a
> ("mm/vmscan: protect the workingset on anonymous LRU")?

I don't think so.  This is only for executable folios (vm_flags &
VM_EXEC).  The behavior for all other anonymous pages are not affected.

Best Regards,
Huang, Ying

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[Miaohe Lin]

On 2022/4/26 10:02, ying.huang@intel.com wrote:
> On Mon, 2022-04-25 at 14:07 -0700, Andrew Morton wrote:
>> On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>>> We should activate swap-backed executable folios (e.g. tmpfs) after first
>>> usage so that executable code gets yet better chance to stay in memory.
>>
>> Does this not erase the intended effect of Joonsoo's b518154e59a
>> ("mm/vmscan: protect the workingset on anonymous LRU")?
> 
> I don't think so.  This is only for executable folios (vm_flags &
> VM_EXEC).  The behavior for all other anonymous pages are not affected.
> 

Agree. Only executable folios are changed! Thanks both!

> Best Regards,
> Huang, Ying
> 
> .
> 

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[Joonsoo Kim]

Hello,

2022년 4월 26일 (화) 오후 3:58, Miaohe Lin <linmiaohe@huawei.com>님이 작성:
>
> On 2022/4/26 10:02, ying.huang@intel.com wrote:
> > On Mon, 2022-04-25 at 14:07 -0700, Andrew Morton wrote:
> >> On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> >>
> >>> We should activate swap-backed executable folios (e.g. tmpfs) after first
> >>> usage so that executable code gets yet better chance to stay in memory.

Missing activation for swap-backed executable page is the intended operation.
So, I disagree with this patch until some numbers are provided.
See following discussion.

https://lore.kernel.org/all/20200316161208.GB67986@cmpxchg.org/T/#u

Thanks.

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[ying.huang]

On Wed, 2022-04-27 at 13:08 +0900, Joonsoo Kim wrote:
> Hello,
> 
> 2022년 4월 26일 (화) 오후 3:58, Miaohe Lin <linmiaohe@huawei.com>님이 작성:
> > 
> > On 2022/4/26 10:02, ying.huang@intel.com wrote:
> > > On Mon, 2022-04-25 at 14:07 -0700, Andrew Morton wrote:
> > > > On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> > > > 
> > > > > We should activate swap-backed executable folios (e.g. tmpfs) after first
> > > > > usage so that executable code gets yet better chance to stay in memory.
> 
> Missing activation for swap-backed executable page is the intended operation.
> So, I disagree with this patch until some numbers are provided.
> See following discussion.
> 
> https://lore.kernel.org/all/20200316161208.GB67986@cmpxchg.org/T/#u

Thanks for your pointer.  Now I understood more about this.  I agree to
keep the original behavior unless we can prove the change with numbers.

Best Regards,
Huang, Ying

Re: [PATCH v3 3/6] mm/vmscan: activate swap-backed executable folios after first usage

[Miaohe Lin]

On 2022/4/27 14:10, ying.huang@intel.com wrote:
> On Wed, 2022-04-27 at 13:08 +0900, Joonsoo Kim wrote:
>> Hello,
>>
>> 2022년 4월 26일 (화) 오후 3:58, Miaohe Lin <linmiaohe@huawei.com>님이 작성:
>>>
>>> On 2022/4/26 10:02, ying.huang@intel.com wrote:
>>>> On Mon, 2022-04-25 at 14:07 -0700, Andrew Morton wrote:
>>>>> On Mon, 25 Apr 2022 19:12:29 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
>>>>>
>>>>>> We should activate swap-backed executable folios (e.g. tmpfs) after first
>>>>>> usage so that executable code gets yet better chance to stay in memory.
>>
>> Missing activation for swap-backed executable page is the intended operation.
>> So, I disagree with this patch until some numbers are provided.
>> See following discussion.
>>
>> https://lore.kernel.org/all/20200316161208.GB67986@cmpxchg.org/T/#u
> 
> Thanks for your pointer.  Now I understood more about this.  I agree to
> keep the original behavior unless we can prove the change with numbers.
> 

Many thanks both! I will keep the original behavior. :)

> Best Regards,
> Huang, Ying
> 
> 
> .
>