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

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

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)

  VM_WARN_ON_ONCE_PAGE(__PageMovable(page), page);


> 
> But now I realized something I previously missed: We are only modifying 
> behavior of hugetlb folios ... stupid misleading "PageHuge" check :)
> 
> So that would be independent of this change.
> 
> Acked-by: David Hildenbrand <david@redhat.com>
>