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

[Zi Yan <ziy@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2968 bytes --]

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.


--
Best Regards,
Yan, Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]