[PATCH] mm/workingset: don't count as refault (file/anon) if refault distance is too long

[PATCH] mm/workingset: don't count as refault (file/anon) if refault distance is too long

[Yongmei Xie]

The current implementation count all the pages which have shadow entry in radix tree as
the event of refault (page or anon). That means all the pages which have ever been
cached in lru are counted as refault. But I think it's not 100% percent correct.

Because usually inode has much longer life cycle than the pages belonging to it.
So if the inode was accessed from time to time, then it won't be reclaimed unless much
severe memory pressure happens.

Then if page was reclaimed several days ago, when it's accessed again, it will be marked as
refault event. Then page was reclaimed an hour ago, when it's accessed again, it will be
marked as refault event as well. From the refault metric alone, I cannot tell whether the
previous reclaim process has evicted some useful pages. That makes things worse in situation
of proactive reclaim. We's like to known whehter the previous policy reclaim too much
meaningful data other than working set transition. So we'd like see the refault rate, but it
includes all the historical reclaim.

From my point of view, if the refaut distance is larger than file cache size, we don't
have to count it refault at all. Because it's too long to be memorized down.

Signed-off-by: Yongmei Xie <yongmeixie@hotmail.com>
---
 mm/workingset.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/workingset.c b/mm/workingset.c
index d4268d8e9a82..fa78f064bd16 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -288,6 +288,7 @@ void workingset_refault(struct page *page, void *shadow)
 	struct lruvec *eviction_lruvec;
 	unsigned long refault_distance;
 	unsigned long workingset_size;
+	unsigned long file_lru_size;
 	struct pglist_data *pgdat;
 	struct mem_cgroup *memcg;
 	unsigned long eviction;
@@ -350,6 +351,11 @@ void workingset_refault(struct page *page, void *shadow)
 	memcg = page_memcg(page);
 	lruvec = mem_cgroup_lruvec(memcg, pgdat);
 
+	file_lru_size = lruvec_page_state(eviction_lruvec, NR_ACTIVE_FILE) +
+			lruvec_page_state(eviction_lruvec, NR_INACTIVE_FILE);
+	if (refault_distance > file_lru_size)
+		goto out;
+
 	inc_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file);
 
 	/*
-- 
2.18.2

Re: [PATCH] mm/workingset: don't count as refault (file/anon) if refault distance is too long

[Andrew Morton]

On Sun, 12 Sep 2021 19:54:47 +0800 Yongmei Xie <yongmeixie@hotmail.com> wrote:

> The current implementation count all the pages which have shadow entry in radix tree as
> the event of refault (page or anon). That means all the pages which have ever been
> cached in lru are counted as refault. But I think it's not 100% percent correct.
> 
> Because usually inode has much longer life cycle than the pages belonging to it.
> So if the inode was accessed from time to time, then it won't be reclaimed unless much
> severe memory pressure happens.
> 
> Then if page was reclaimed several days ago, when it's accessed again, it will be marked as
> refault event. Then page was reclaimed an hour ago, when it's accessed again, it will be
> marked as refault event as well. From the refault metric alone, I cannot tell whether the
> previous reclaim process has evicted some useful pages. That makes things worse in situation
> of proactive reclaim. We's like to known whehter the previous policy reclaim too much
> meaningful data other than working set transition. So we'd like see the refault rate, but it
> includes all the historical reclaim.
> 
> >From my point of view, if the refaut distance is larger than file cache size, we don't
> have to count it refault at all. Because it's too long to be memorized down.

Thanks.

Do you have any runtime testing results which demonstrate a benefit?