Re: [PATCH] mm: remove migration for HugePage in isolate_single_pageblock()

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

On 2024/8/18 7:58, Zi Yan wrote:
> On 17 Aug 2024, at 2:13, Kefeng Wang wrote:
> 
>> On 2024/8/17 3:45, David Hildenbrand wrote:
>>> On 16.08.24 13:30, Kefeng Wang wrote:
>>>>
>>>>
>>>> On 2024/8/16 18:11, David Hildenbrand wrote:
>>>>> On 16.08.24 06:06, Kefeng Wang wrote:
>>>>>> The gigantic page size may larger than memory block size, so memory
>>>>>> offline always fails in this case after commit b2c9e2fbba32 ("mm: make
>>>>>> alloc_contig_range work at pageblock granularity"),
>>>>>>
>>>>>> offline_pages
>>>>>>      start_isolate_page_range
>>>>>>        start_isolate_page_range(isolate_before=true)
>>>>>>          isolate [isolate_start, isolate_start + pageblock_nr_pages)
>>>>>>        start_isolate_page_range(isolate_before=false)
>>>>>>          isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock
>>>>>>               __alloc_contig_migrate_range
>>>>>>              isolate_migratepages_range
>>>>>>                isolate_migratepages_block
>>>>>>                  isolate_or_dissolve_huge_page
>>>>>>                    if (hstate_is_gigantic(h))
>>>>>>                        return -ENOMEM;
>>>>>>
>>>>>> In fact, we don't need to migrate page in page range isolation, for
>>>>>> memory offline path, there is do_migrate_range() to move the pages.
>>>>>> For contig allocation, there is another __alloc_contig_migrate_range()
>>>>>> after isolation to migrate the pages. So fix issue by skipping the
>>>>>> __alloc_contig_migrate_range() in isolate_single_pageblock().
>>
>> ...
>>
>>>
>>> Please distill some of that in the patch description. Right now you only talk about memory offlining and don't cover why alloc_contig_range() is fine as well with this change.
>>
>> Borrowing some word from Zi,
>>
>>
>> PageHuge(gigantic) can bigger than a pageblock, the gigantic PageHuge is freed as order-0. This means MIGRATE_ISOLATE pageblocks will get to the right free list after __alloc_contig_migrate_range(), the one after
>> start_isolate_page_range() for alloc_contig_range(), this is same as in
>> memory offline, it has own path to isolate/migrate used page and dissolve the free hugepages, so the migration code in isolate_single_pageblock() is not needed, let's cleanup it and which also fix the above the issue.
>>
>> Please correct me or help to write better description, thanks.
> 
> How about?
> 
> Gigantic PageHuge is bigger than a pageblock, but since it is freed as order-0 pages,
> its pageblocks after being freed will get to the right free list. There is no need
> to have special handling code for them in start_isolate_page_range(). For both
> alloc_contig_range() and memory offline cases, the migration code after
> start_isolate_page_range() will be able to migrate gigantic PageHuge when possible.
> Let's clean up start_isolate_page_range() and fix the aforementioned memory offline
> failure issue all together.

Thanks Zi, it is better, will update.
> 
> 
> --
> Best Regards,
> Yan, Zi