Re: [RFC PATCH 2/3] mm: Create snapshot_page()

[Shivank Garg <shivankg@xxxxxxx>]

Hi Willy,

On 2/11/2025 2:51 AM, Matthew Wilcox (Oracle) wrote:

< snip >
...

> +const struct folio *snapshot_page(struct folio *foliop, struct page *precise,
> +		unsigned long *idxp, const struct page *unstable)
> +{
> +	struct folio *folio;
> +	unsigned long head;
> +	unsigned long idx, nr_pages = 1;
> +	int loops = 5;
> +
> +again:
> +	memcpy(precise, unstable, sizeof(struct page));
> +	head = precise->compound_head;
> +	/* Open-coded !PageTail because page_is_fake_head() doesn't work here */
> +	if ((head & 1) == 0) {
> +		folio = (struct folio *)precise;
> +		*idxp = 0;
> +		/* Not a tail, not a head, we have a single page */
> +		if (!folio_test_large(folio))
> +			goto out;
> +		folio = (struct folio *)unstable;
> +	} else {
> +		folio = (struct folio *)(head - 1);
> +		*idxp = folio_page_idx(folio, unstable);
> +	}
> +
> +	if (idx < MAX_FOLIO_NR_PAGES || folio_test_hugetlb(folio)) {

idx is not initialized before use. I think you meant *idxp here.

> +		memcpy(foliop, folio, sizeof(struct folio));
> +		nr_pages = folio_nr_pages(foliop);
> +		folio = foliop;
> +	}
> +
> +	if (idx > nr_pages) {
> +		if (loops-- > 0)
> +			goto again;
> +		pr_warn("page does not match folio\n");
> +		precise->compound_head &= ~1UL;
> +		folio = (struct folio *)precise;
> +		*idxp = 0;
> +	}
> +out:
> +	return folio;
> +}

Please consider adding my

Reviewed-by: Shivank Garg <shivankg@xxxxxxx>

with this fix:

diff --git a/mm/util.c b/mm/util.c
index 9f9cf3933eb1..155493b71d28 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1253,7 +1253,7 @@ const struct folio *snapshot_page(struct folio *foliop, struct page *precise,
 {
 	struct folio *folio;
 	unsigned long head;
-	unsigned long idx, nr_pages = 1;
+	unsigned long nr_pages = 1;
 	int loops = 5;
 
 again:
@@ -1272,13 +1272,13 @@ const struct folio *snapshot_page(struct folio *foliop, struct page *precise,
 		*idxp = folio_page_idx(folio, unstable);
 	}
 
-	if (idx < MAX_FOLIO_NR_PAGES || folio_test_hugetlb(folio)) {
+	if (*idxp < MAX_FOLIO_NR_PAGES || folio_test_hugetlb(folio)) {
 		memcpy(foliop, folio, sizeof(struct folio));
 		nr_pages = folio_nr_pages(foliop);
 		folio = foliop;
 	}
 
-	if (idx > nr_pages) {
+	if (*idxp > nr_pages) {
 		if (loops-- > 0)
 			goto again;
 		pr_warn("page does not match folio\n");