Re: [PATCH] mm: shmem: enable thp migration (Re: [PATCH v1] mm: consider non-anonymous thp as unmovable page)

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Fri, Apr 06, 2018 at 03:07:11AM +0000, Naoya Horiguchi wrote:
> Subject: [PATCH] mm: enable thp migration for shmem thp

This patch is buggy, but not in a significant way:

> @@ -524,13 +524,26 @@ int migrate_page_move_mapping(struct address_space *mapping,
>  	}
>  
>  	radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);

^^^ this line should have been deleted

> +	if (PageTransHuge(page)) {
> +		int i;
> +		int index = page_index(page);
> +
> +		for (i = 0; i < HPAGE_PMD_NR; i++) {
^^^ or this iteration should start at 1
> +			pslot = radix_tree_lookup_slot(&mapping->i_pages,
> +						       index + i);
> +			radix_tree_replace_slot(&mapping->i_pages, pslot,
> +						newpage + i);
> +		}
> +	} else {
> +		radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
^^^ and if the second option, then we don't need this line
> +	}

So either this:

-	radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
+	if (PageTransHuge(page)) {
+		int i;
+		int index = page_index(page);
+
+		for (i = 0; i < HPAGE_PMD_NR; i++) {
+			pslot = radix_tree_lookup_slot(&mapping->i_pages,
+						       index + i);
+			radix_tree_replace_slot(&mapping->i_pages, pslot,
+						newpage + i);
+		}
+	} else {
+		radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
+	}

Or this:

 	radix_tree_replace_slot(&mapping->i_pages, pslot, newpage);
+	if (PageTransHuge(page)) {
+		int i;
+		int index = page_index(page);
+
+		for (i = 1; i < HPAGE_PMD_NR; i++) {
+			pslot = radix_tree_lookup_slot(&mapping->i_pages,
+						       index + i);
+			radix_tree_replace_slot(&mapping->i_pages, pslot,
+						newpage + i);
+		}
+	}

The second one is shorter and involves fewer lookups ...