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

[Jiaqi Yan]

On Fri, Jul 17, 2026 at 12:37 AM Miaohe Lin <linmiaohe@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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
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.

>
>         /*
>          * 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().

>         }
>
> But __update_and_free_hugetlb_folio has no return value. So caller can't know exactly
> whether hugetlb folio is indeed dissolved. So maybe we need a way to detect whether
> hugetlb folio is indeed dissolved?
>
> Note before this patch, take_page_off_buddy will fail if hugetlb folio is not dissolved.
>
> Thanks.
> .
>