Re: [PATCH 2/4] mm: memory_hotplug: check hwpoisoned page firstly in do_migrate_range()

[David Hildenbrand <david@redhat.com>]

On 02.08.24 09:50, Kefeng Wang wrote:
> 
> 
> On 2024/8/2 4:10, David Hildenbrand wrote:
>>
>>>>
>>>> We're not checking the head page here, will this work reliably for
>>>> hugetlb? (I recall some difference in per-page hwpoison handling between
>>>> hugetlb and THP due to the vmemmap optimization)
>>>
>>> Before this changes, the hwposioned hugetlb page won't try to unmap in
>>> do_migrate_range(), we hope it already unmapped in memory_failure(), as
>>> mentioned from comments, there maybe fail to unmap, so a new safeguard
>>> to try to unmap it again here, but we don't need to guarantee it.
>>
>> Thanks for clarifying!
>>
>> But I do wonder if the PageHWPoison() is the right thing to do for hugetlb.
>>
>> IIUC, hugetlb requires folio_test_hwpoison() -- testing the head page
>> not the subpage. Reason is that due to the vmemmap optimization we might
>> not be able to modify subpages to set hwpoison.
> 
> Yes, HVO is special(only head page with hwpoison), since we always want
> to check head page here (next pfn = head_pfn + nr), so it might be
> enough to only use PageHWpoison, but just in case, adding hwpoison check
> for the head page,
> 
> 	if (unlikely(PageHWPoison(page) || folio_test_hwpoison(folio)))

I also do wonder if we have to check for large folios 
folio_test_has_hwpoison():
if any subpage is poisoned, not just the current page.

I don't think folio_set_has_hwpoisoned() is used for hugetlb.

What a mess.


-- 
Cheers,

David / dhildenb