Re: [PATCH] mm/memory_hotplug: fix call folio_test_large with tail page in do_migrate_range

[David Hildenbrand <david@redhat.com>]

On 29.03.25 00:37, Andrew Morton wrote:
> On Tue, 25 Mar 2025 20:05:50 +0100 David Hildenbrand <david@redhat.com> wrote:
> 
>> On 25.03.25 04:02, Jinjiang Tu wrote:
>>>
>>>> Moving that will not make it able to skip the large frozen
>>>> (refcount==0, e.g., free hugetlb) folio in the continue/put_folio case
>>>> above. Hmmmm ..
>>> For free hugetlb, pfn is increased by 1 in each loop. This leads to skip
>>> free hugetlb slower.
>>
>> Yes. But now I realize that we have the same issue with free buddy pages
>> already (folio_try_get of each individual page :( ).
>>
>>>>
>>>> We could similarly to dumping folios, snapshot them, so we can read
>>>> stable data.
>>> extract the code in __dump_page()? But snapshot may lead to
>>> do_migrate_range() slower too.
>>
>> There is a patch series on the list to do that, but it might take a
>> while to clean that up. Ideally, we'd also jump over free buddy pages.
>> In the future we might have better ways to do that.
>>
>> I don't consider this change here really important, but if all it
>> affects is free hugetlb folios, it's not really worth it to have this
>> code around.
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>>
>> But: I suspect 8135d8926c08 is not the introducing commit. Please re-verify.
>>
>> We should not CC stable.
> 

Sorry for the late reply.

> Why shouldn't we cc:stable?  A userspace-triggerable BUG?

My reasoning was that it's a VM_BUG_ON_PAGE, and nothing is actually 
broken on !CONFIG_DEBUG_VM systems ... combined with hugetlb allocation 
and memory offlining usually being rare and usually privileged operations.

It would be a different story if the VM_BUG_ON_PAGE revealed an actual 
issue; but this is a well documented race.

One might argue that unplugging a DIMM could automatically trigger it 
(rare operation ..), and as we discovered in the meantime, THPs are also 
affected. But again, CONFIG_VM_DEBUG ... which even Fedora stopped setting.

So I don't think CC stable is warranted here, but I wouldn't object if 
there are good reasons to do it: like assuming that there are 
CONFIG_VM_DEBUG stable users out there that rely on these features.

-- 
Cheers,

David / dhildenb