Re: [PATCH v6 4/5] mm/memory-failure: skip take_page_off_buddy after dissolving HWPoison HugeTLB page

[Miaohe Lin <linmiaohe@huawei.com>]

On 2026/8/17 8:29, Jiaqi Yan wrote:
> On Fri, Jul 17, 2026 at 12:37 AM Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> On 2026/7/6 2:07, Jiaqi Yan wrote:
>>> Now that HWPoison subpage(s) within HugeTLB page will be rejected by
>>> buddy allocator during dissolve_free_hugetlb_folio(), there is no
>>> need to drain_all_pages() and take_page_off_buddy() anymore. In fact,
>>> calling take_page_off_buddy() after dissolve_free_hugetlb_folio()
>>> succeeded returns false, making caller think __page_handle_poison()
>>> failed.
>>>
>>> Add __hugepage_handle_poison() and replace __page_handle_poison() at
>>> HugeTLB specific call sites. The being handled HugeTLB page either
>>> is free at the moment of try_memory_failure_hugetlb(), or becomes
>>> free at the moment of me_huge_page().
>>>
>>> Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>
>>> ---
>>>  mm/memory-failure.c | 36 ++++++++++++++++++++++++++++++------
>>>  1 file changed, 30 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>> index 3d15b4c1b694..a37b67550718 100644
>>> --- a/mm/memory-failure.c
>>> +++ b/mm/memory-failure.c
>>> @@ -174,6 +174,30 @@ static struct rb_root_cached pfn_space_itree = RB_ROOT_CACHED;
>>>  static DEFINE_MUTEX(pfn_space_lock);
>>>
>>>  /*
>>> + * Only for a HugeTLB page being handled by memory_failure(). The key
>>> + * difference to soft_offline() is that, no HWPoison subpage will make
>>> + * into buddy allocator after a successful dissolve_free_hugetlb_folio(),
>>> + * so take_page_off_buddy() is unnecessary.
>>> + */
>>> +static int __hugepage_handle_poison(struct page *page)
>>> +{
>>> +     struct folio *folio = page_folio(page);
>>> +
>>> +     /*
>>> +      * Can't use dissolve_free_hugetlb_folio() without a reliable
>>> +      * raw_hwp_list telling which subpage is HWPoison. So do not free
>>> +      * them to the buddy allocator. dequeue_hugetlb_folio_node_exact()
>>> +      * will ensure to never re-allocate this hugepage.
>>> +      */
>>> +     if (folio_test_hugetlb_raw_hwp_unreliable(folio))
>>> +             /* raw_hwp_list becomes unreliable when kmalloc() fails. */
>>> +             return -ENOMEM;
>>
>> There are some branches in __update_and_free_hugetlb_folio that will leave hugetlb
>> folio untouched:
>>
>> static void __update_and_free_hugetlb_folio(struct hstate *h,
>>                                                 struct folio *folio)
>> {
>>         bool clear_flag = folio_test_hugetlb_vmemmap_optimized(folio);
>>
>>         if (hstate_is_gigantic_no_runtime(h))
>>                 return;<-- 1
> 
> Thanks for catching this, Miaohe.
> 
> I think the most challenging part is that
> update_and_free_hugetlb_folio() must support deferring freeing (via
> schedule_work()), so adding a return value isn't that straightforward
> without some refactoring...
> 
> If making __hugepage_handle_poison() check
> hstate_is_gigantic_no_runtime() == 0 (or
> gigantic_page_runtime_supported() == 1) isn't an absurd idea, we can

I'm afraid this might not be a good idea. Maybe we could re-check page state after
calling dissolve_free_hugetlb_folio?

> just do that and avoid adding return value to
> __update_and_free_hugetlb_folio().
> 
>>
>>         /*
>>          * If we don't know which subpages are hwpoisoned, we can't free
>>          * the hugepage, so it's leaked intentionally.
>>          */
>>         if (folio_test_hugetlb_raw_hwp_unreliable(folio))
>>                 return;<-- 2
> 
> __hugepage_handle_poison() already checked this, and with mf_mutex no
> one can set raw_hwp_unreliable.

Agreed.

> 
>>
>>         /*
>>          * If folio is not vmemmap optimized (!clear_flag), then the folio
>>          * is no longer identified as a hugetlb page.  hugetlb_vmemmap_restore_folio
>>          * can only be passed hugetlb pages and will BUG otherwise.
>>          */
>>         if (clear_flag && hugetlb_vmemmap_restore_folio(h, folio)) {
>>                 spin_lock_irq(&hugetlb_lock);
>>                 /*
>>                  * If we cannot allocate vmemmap pages, just refuse to free the
>>                  * page and put the page back on the hugetlb free list and treat
>>                  * as a surplus page.
>>                  */
>>                 add_hugetlb_folio(h, folio, true);
>>                 spin_unlock_irq(&hugetlb_lock);
>>                 return;<-- 3
> 
> __hugepage_handle_poison() should not get into this if-block because
> dissolve_free_hugetlb_folio() must have
> hugetlb_vmemmap_restore_folio()-ed successfully, so clear_flag must be
> false here. Otherwise dissolve_free_hugetlb_folio() already returns
> early without update_and_free_hugetlb_folio().

Agreed.

Thanks.
.