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

[David Hildenbrand <david@redhat.com>]

On 07.04.25 03:59, Andrew Morton wrote:
> On Tue, 1 Apr 2025 18:44:06 +0200 David Hildenbrand <david@redhat.com> wrote:
> 
>> 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.
> 
> Oh.  I thought that quite a few distros use CONFIG_DEBUG_VM.

Fedora used to be the prime example people kept mentioning. But they 
stopped using it for performance reasons IIRC.

-- 
Cheers,

David / dhildenb