Received-SPF: pass (google.com: domain of linux-kernel+bounces-202041-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 5 Jun 2024 01:55:17 -0700
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Usama Arif <usamaarif642@gmail.com>
Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, willy@infradead.org, 
	yosryahmed@google.com, nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH v2 1/2] mm: clear pte for folios that are zero filled
Message-ID: <nes73bwc5p6yhwt5tw3upxcqrn5kenn6lvqb6exrf4yppmz6jx@ywhuevpkxlvh>
References: <20240604105950.1134192-1-usamaarif642@gmail.com>
 <20240604105950.1134192-2-usamaarif642@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240604105950.1134192-2-usamaarif642@gmail.com>

On Tue, Jun 04, 2024 at 11:58:24AM GMT, Usama Arif wrote:
[...]
>  
> +static bool is_folio_page_zero_filled(struct folio *folio, int i)
> +{
> +	unsigned long *data;
> +	unsigned int pos, last_pos = PAGE_SIZE / sizeof(*data) - 1;
> +	bool ret = false;
> +
> +	data = kmap_local_folio(folio, i * PAGE_SIZE);
> +
> +	if (data[last_pos])
> +		goto out;
> +

Use memchr_inv() instead of the following.

> +	for (pos = 0; pos < last_pos; pos++) {
> +		if (data[pos])
> +			goto out;
> +	}
> +	ret = true;
> +out:
> +	kunmap_local(data);
> +	return ret;
> +}
> +
[...]
> +
>  /*
>   * shrink_folio_list() returns the number of reclaimed pages
>   */
> @@ -1053,6 +1085,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>  		enum folio_references references = FOLIOREF_RECLAIM;
>  		bool dirty, writeback;
>  		unsigned int nr_pages;
> +		bool folio_zero_filled = false;
>  
>  		cond_resched();
>  
> @@ -1270,6 +1303,7 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>  			nr_pages = 1;
>  		}
>  
> +		folio_zero_filled = is_folio_zero_filled(folio);

You need to check for zeroes after the unmap below otherwise you may
lost data. So you need to do two rmap walks. Most probably the first one
would be the standard one (inserting swap entry in the ptes) but the
second one would be different where swap entries should be replaced by
the zeropage. Also at the end you need to make sure to release all the
swap resources associated with the given page/folio.

>  		/*
>  		 * The folio is mapped into the page tables of one or more
>  		 * processes. Try to unmap it here.
> @@ -1295,6 +1329,9 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>  			if (folio_test_large(folio) && list_empty(&folio->_deferred_list))
>  				flags |= TTU_SYNC;
>  
> +			if (folio_zero_filled)
> +				flags |= TTU_ZERO_FOLIO;
> +
>  			try_to_unmap(folio, flags);
>  			if (folio_mapped(folio)) {
>  				stat->nr_unmap_fail += nr_pages;