Re: [PATCH v3 5-fix/5] mm: memory_hotplug: unify Huge/LRU/non-LRU movable folio isolation fix

[David Hildenbrand <david@xxxxxxxxxx>]

On 29.08.24 17:05, Kefeng Wang wrote:
>    - Use folio_try_get()/folio_put() unconditionally, per David
>    - print current pfn when isolate_folio_to_list fails.
>    - memory hotremove test passed for free/used hugetlb folio
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> ---
>   mm/memory_hotplug.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 5f09866d17cf..621ae1015106 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1780,7 +1780,6 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>   
>   	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>   		struct page *page;
> -		bool hugetlb;
>   
>   		if (!pfn_valid(pfn))
>   			continue;
> @@ -1811,22 +1810,21 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>   			continue;
>   		}
>   
> -		hugetlb = folio_test_hugetlb(folio);
> -		if (!hugetlb) {
> -			folio = folio_get_nontail_page(page);
> -			if (!folio)
> -				continue;
> -		}
> +		if (!folio_try_get(folio))
> +			continue;
> +
> +		if (unlikely(page_folio(page) != folio))
> +			goto put_folio;
>   
>   		if (!isolate_folio_to_list(folio, &source)) {
>   			if (__ratelimit(&migrate_rs)) {
> -				pr_warn("failed to isolate pfn %lx\n", pfn);
> +				pr_warn("failed to isolate pfn %lx\n",
> +					page_to_pfn(page));
>   				dump_page(page, "isolation failed");
>   			}
>   		}
> -
> -		if (!hugetlb)
> -			folio_put(folio);
> +put_folio:
> +		folio_put(folio);
>   	}
>   	if (!list_empty(&source)) {
>   		nodemask_t nmask = node_states[N_MEMORY];

LGTM, isolate_hugetlb() would not handle free hugetlb folios either, 
because of the folio_try_get().

--
Cheers,

David / dhildenb