Received-SPF: pass (google.com: domain of linux-kernel+bounces-202144-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: alexs@kernel.org
To: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	izik.eidus@ravellosystems.com,
	willy@infradead.org,
	aarcange@redhat.com,
	chrisw@sous-sol.org,
	hughd@google.com,
	david@redhat.com
Cc: "Alex Shi (tencent)" <alexs@kernel.org>
Subject: [RFC 3/3] mm/ksm: move flush_anon_page before checksum calculation
Date: Wed,  5 Jun 2024 17:53:03 +0800
Message-ID: <20240605095304.66389-3-alexs@kernel.org>
In-Reply-To: <20240605095304.66389-1-alexs@kernel.org>
References: <20240605095304.66389-1-alexs@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: "Alex Shi (tencent)" <alexs@kernel.org>

commit 6020dff09252 ("[ARM] Resolve fuse and direct-IO failures due to missing cache flushes")
explain that the aim of flush_anon_page() is to keep the cache and memory
content synced. Also as David Hildenbrand pointed, flush page without
the page contents reading here is meaningless, so let's move the flush action
just before page contents reading, like calc_checksum(), not
just find a page, flush it, w/o clear purpose. This should save some flush
actions why keep page content safely synced.

BTW, write_protect_page() do another type flush actions before pages_identical().

Signed-off-by: Alex Shi (tencent) <alexs@kernel.org>
---
 mm/ksm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index ef335ee508d3..77e8c1ded9bb 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -784,10 +784,7 @@ static struct page *get_mergeable_page(struct ksm_rmap_item *rmap_item)
 		goto out;
 	if (is_zone_device_page(page))
 		goto out_putpage;
-	if (PageAnon(page)) {
-		flush_anon_page(vma, page, addr);
-		flush_dcache_page(page);
-	} else {
+	if (!PageAnon(page)) {
 out_putpage:
 		put_page(page);
 out:
@@ -2378,7 +2375,12 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
 		mmap_read_unlock(mm);
 		return;
 	}
+
+	/* flush page contents before calculate checksum */
+	flush_anon_page(vma, page, rmap_item->address);
+	flush_dcache_page(page);
 	checksum = calc_checksum(page);
+
 	if (rmap_item->oldchecksum != checksum) {
 		rmap_item->oldchecksum = checksum;
 		mmap_read_unlock(mm);
@@ -2662,8 +2664,6 @@ static struct ksm_rmap_item *scan_get_next_rmap_item(struct page **page)
 			if (is_zone_device_page(*page))
 				goto next_page;
 			if (PageAnon(*page)) {
-				flush_anon_page(vma, *page, ksm_scan.address);
-				flush_dcache_page(*page);
 				rmap_item = get_next_rmap_item(mm_slot,
 					ksm_scan.rmap_list, ksm_scan.address);
 				if (rmap_item) {
-- 
2.43.0