Received-SPF: pass (google.com: domain of linux-kernel+bounces-7042-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 20 Dec 2023 14:14:05 +0000
From: Matthew Wilcox <willy@infradead.org>
To: "zhaoyang.huang" <zhaoyang.huang@unisoc.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Zhaoyang Huang <huangzhaoyang@gmail.com>, steve.kang@unisoc.com
Subject: Re: [RFC PATCH 1/1] mm: mark folio accessed in minor fault
Message-ID: <ZYL2rbD5UTz3s8gg@casper.infradead.org>
References: <20231220102948.1963798-1-zhaoyang.huang@unisoc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231220102948.1963798-1-zhaoyang.huang@unisoc.com>

On Wed, Dec 20, 2023 at 06:29:48PM +0800, zhaoyang.huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> Inactive mapped folio will be promoted to active only when it is
> scanned in shrink_inactive_list, while the vfs folio will do this
> immidiatly when it is accessed. These will introduce two affections:
> 
> 1. NR_ACTIVE_FILE is not accurate as expected.
> 2. Low reclaiming efficiency caused by dummy nactive folio which should
>    be kept as earlier as shrink_active_list.
> 
> I would like to suggest mark the folio be accessed in minor fault to
> solve this situation.

This isn't going to be as effective as you imagine.  Almost all file
faults are handled through filemap_map_pages().  So I must ask, what
testing have you done with this patch?

And while you're gathering data, what effect would this patch have on your
workloads?

diff --git a/mm/filemap.c b/mm/filemap.c
index 2e6b1daac6cd..8cecf82dcc5a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3418,6 +3418,7 @@ static struct folio *next_uptodate_folio(struct xa_state *xas,
 		max_idx = DIV_ROUND_UP(i_size_read(mapping->host), PAGE_SIZE);
 		if (xas->xa_index >= max_idx)
 			goto unlock;
+		folio_mark_accessed(folio);
 		return folio;
 unlock:
 		folio_unlock(folio);