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

On Mon, Aug 17, 2026 at 12:23 AM Miaohe Lin <[email protected]> wrote:
>
> On 2026/8/17 8:29, Jiaqi Yan wrote:
> > On Fri, Jul 17, 2026 at 12:37 AM Miaohe Lin <[email protected]> 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 <[email protected]>
> >>> ---
> >>>  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

Any specific reason? It seems to me that code that included
"hugetlb.h" can access gigantic_page_runtime_supported().

I feel checking gigantic_page_runtime_supported() upfront is cleaner
than checking page state after dissolve:

if (!gigantic_page_runtime_supported())
    return -EOPNOTSUPP;

vs

/* Direct state check after dissolve */
rc = dissolve_free_hugetlb_folio(folio);
if (!rc && folio_test_hugetlb(folio)) {
    /* Dissolve failed silently due to unsupported gigantic runtime freeing */
    rc = -EOPNOTSUPP;
}


> 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.
> .