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

[Jiaqi Yan]

On Tue, Aug 18, 2026 at 2:06 AM Miaohe Lin <linmiaohe@xxxxxxxxxx> wrote:
>
> On 2026/8/18 11:30, Jiaqi Yan wrote:
> > On Mon, Aug 17, 2026 at 12:23 AM Miaohe Lin <linmiaohe@xxxxxxxxxx> wrote:
> >>
> >> On 2026/8/17 8:29, Jiaqi Yan wrote:
> >>> 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
> >>
> >> 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 am concerned that with the changes of the implementation of
> dissolve_free_hugetlb_folio(), gigantic_page_runtime_supported()
> might not be the sole reason causing dissolve_free_hugetlb_folio()
> to fail. This would require __hugepage_handle_poison() to be modified
> accordingly but we might miss that in the future... But still no strong
> opinion here. :)

Coming back from a long vacation and catching up with this conversation....

I found commit a363c62a653c ("mm/hugetlb: do not dissolve gigantic
pages without runtime support") has landed in akpm/mm-unstable, adding
"if (hstate_is_gigantic_no_runtime(h)) goto out;" directly inside
dissolve_free_hugetlb_folio(). So for this particular case, I think no
more checking is necessary.

Your concern in general still holds. Usually or at least sometimes,
caller of dissolve_free_hugetlb_folio() expects its return value
reflects the true results of dissolving and freeing; however,
update_and_free_hugetlb_folio() can bail out silently.

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