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

[David Hildenbrand <david@redhat.com>]

On 20.08.24 16:00, Kefeng Wang wrote:
> 
> 
> On 2024/8/20 16:42, David Hildenbrand wrote:
>> On 20.08.24 05:26, 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;
>>>
>>> [   15.815756] memory offlining [mem 0x3c0000000-0x3c7ffffff] failed
>>> due to failure to isolate range
>>>
>>> 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.
>>>
>>> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock
>>> granularity")
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
> ...
> 
>>> +            if (head_pfn + nr_pages <= boundary_pfn ||
>>> +                PageHuge(page)) {
>>
>> I'm wondering if we should have here some kind of WARN_ON_ONCE if
>> PageLRU + "spans more than a single pageblock" check.
>>
>> Then we could catch whenever we would have !hugetlb LRU folios that span
>> more than a single pageblock.
>>
>> /*
>>    * We cannot currently handle movable (LRU) folios that span more than
>>    * a single pageblock. hugetlb folios are fine, though.
>>    */
>> WARN_ON_ONCE(PageLRU(page) && nr_pages > pageblock_nr_pages
> 
> This should be already covered by following VM_WRAN,
> 
>    VM_WARN_ON_ONCE_PAGE(PageLRU(page), page); // only hint when head_pfn
> + nr_pages > boundary_pfn ( boundary_pfn is pageblock aligned)

Ahh, good!

-- 
Cheers,

David / dhildenb