Received-SPF: pass (google.com: domain of linux-kernel+bounces-202166-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <a1ba41f1-ecff-4790-b85a-ce5dc8077b91@gmail.com>
Date: Wed, 5 Jun 2024 18:04:19 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC 3/3] mm/ksm: move flush_anon_page before checksum
 calculation
To: alexs@kernel.org, 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
References: <20240605095304.66389-1-alexs@kernel.org>
 <20240605095304.66389-3-alexs@kernel.org>
Content-Language: en-US
From: Alex Shi <seakeel@gmail.com>
In-Reply-To: <20240605095304.66389-3-alexs@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Let me withdraw this patch, the flush_anon_page come with the first patchset of ksm, and no explanation for them.
Though, no any guarantee for concurrent page write for the flush, but anyway I give up on the flush optimize.

Sorry for disturber.

Alex

On 6/5/24 5:53 PM, alexs@kernel.org wrote:
> 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) {