Re: [PATCH 09/10] mm/ksm: calc_checksum for folio

[David Hildenbrand <david@redhat.com>]

On 04.06.24 06:24, alexs@kernel.org wrote:
> From: "Alex Shi (tencent)" <alexs@kernel.org>
> 
> Let's check the whole folio for contents change, don't count it if the
> folio changed.
> 
> Signed-off-by: Alex Shi (tencent) <alexs@kernel.org>
> ---
>   mm/ksm.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index b9c04ce677b9..dc2b5e6a9659 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -1258,11 +1258,13 @@ static int unmerge_and_remove_all_rmap_items(void)
>   }
>   #endif /* CONFIG_SYSFS */
>   
> -static u32 calc_checksum(struct page *page)
> +static u32 calc_checksum(struct folio *folio)
>   {
>   	u32 checksum;
> -	void *addr = kmap_local_page(page);
> -	checksum = xxhash(addr, PAGE_SIZE, 0);
> +	int nr = folio_nr_pages(folio);
> +	void *addr = kmap_local_page(folio_page(folio, 0));
> +
> +	checksum = xxhash(addr, nr * PAGE_SIZE, 0);
>   	kunmap_local(addr);
>   	return checksum;
>   }
> @@ -2369,7 +2371,7 @@ static void cmp_and_merge_page(struct page *page, struct ksm_rmap_item *rmap_ite
>   	 * don't want to insert it in the unstable tree, and we don't want
>   	 * to waste our time searching for something identical to it there.
>   	 */
> -	checksum = calc_checksum(page);
> +	checksum = calc_checksum(folio);

So for a large folio you suddenly checksum more than a single page? 
That's wrong.

Or am I missing something?


-- 
Cheers,

David / dhildenb