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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2024/8/22 15:20, Miaohe Lin wrote:
> On 2024/8/17 16:49, Kefeng Wang wrote:
>> Use the isolate_folio_to_list() to unify hugetlb/LRU/non-LRU
>> folio isolation, which cleanup code a bit and save a few calls
>> to compound_head().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/memory_hotplug.c | 45 +++++++++++++++++----------------------------
>>   1 file changed, 17 insertions(+), 28 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 02a0d4fbc3fe..cc9c16db2f8c 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1773,14 +1773,14 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
>>   static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>   {
>>   	unsigned long pfn;
>> -	struct page *page;
>>   	LIST_HEAD(source);
>> +	struct folio *folio;
>>   	static DEFINE_RATELIMIT_STATE(migrate_rs, DEFAULT_RATELIMIT_INTERVAL,
>>   				      DEFAULT_RATELIMIT_BURST);
>>   
>>   	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>> -		struct folio *folio;
>> -		bool isolated;
>> +		struct page *page;
>> +		bool huge;
>>   
>>   		if (!pfn_valid(pfn))
>>   			continue;
>> @@ -1812,34 +1812,22 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>   			continue;
>>   		}
>>   
>> -		if (folio_test_hugetlb(folio)) {
>> -			isolate_hugetlb(folio, &source);
>> -			continue;
>> +		huge = folio_test_hugetlb(folio);
>> +		if (!huge) {
>> +			folio = folio_get_nontail_page(page);
>> +			if (!folio)
>> +				continue;
>>   		}
>>   
>> -		if (!get_page_unless_zero(page))
> 
> Does page always equal to head? Page is used in most cases in this function and head is only used to calculate pfn.
> If not, folio can not simply replace page?

Not very clear what your mean, no guarantee for page == head, that
is why we need use get_page_unless_zero or folio_get_nontail_page to get 
the folio, then try to isolate folio, since we only migrate the wholo
folio, I don't know why we can't convert page to folio here.

> 
>> -			continue;
>> -		/*
>> -		 * We can skip free pages. And we can deal with pages on
>> -		 * LRU and non-lru movable pages.
>> -		 */
>> -		if (PageLRU(page))
> 
> I think this check is incorrect. We should check __PageMovable(page) instead. So this patch
> fixes this issue too.
> 
> Thanks.
> .