Re: [PATCH v6 0/5] Only free healthy pages in high-order has_hwpoisoned folio

[Jiaqi Yan]

On Wed, Aug 5, 2026 at 2:10 AM David Hildenbrand (Arm) <david@xxxxxxxxxx> wrote:
>
> On 8/5/26 10:58, Vlastimil Babka (SUSE) wrote:
> > On 8/4/26 21:51, David Hildenbrand (Arm) wrote:
> >> On 7/28/26 19:18, Vlastimil Babka (SUSE) wrote:
> >>>
> >>> We check all kinds of unexpected state, when that's enable. PageHWPoison
> >>> could have been considered unexpected too, when the checks were made more
> >>> and more optional (first by Mel and then me).
> >>>
> >>> But indeed it seems the PageHWPoison check is supposed to be load-bearing
> >>> (hi, Lorenzo!). It's intentionally handled before bad_page() (with a taint)
> >>> in check_new_page_bad(). Commits 2a7684a23e9c and f4c18e6f7b5b are also a hint.
> >>>
> >>>
> >>> Yes, the PageHWPoison handling was made part of debugging sanity check and
> >>> then not recognized properly as load-bearing later.
> >>
> >> BTW, I'd assume we can check for PageHWPoison at free time fairly efficiently, as we
> >> touch all page flags already.
> >>
> >> Something along the lines of:
> >>
> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >> index 520ebd2fa40b5..57f89f8684168 100644
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -1321,6 +1321,7 @@ static __always_inline bool __free_pages_prepare(struct page *page,
> >>         bool init = want_init_on_free();
> >>         bool compound = PageCompound(page);
> >>         struct folio *folio = page_folio(page);
> >> +       unsigned int hwpoisoned = 0;
> >>
> >>         if (fpi_flags & FPI_PREPARED)
> >>                 return true;
> >> @@ -1347,21 +1348,6 @@ static __always_inline bool __free_pages_prepare(struct page *page,
> >>                 count_vm_events(UNEVICTABLE_PGCLEARED, nr_pages);
> >>         }
> >>
> >> -       if (unlikely(PageHWPoison(page)) && !order) {
> >> -               /* Do not let hwpoison pages hit pcplists/buddy */
> >> -               reset_page_owner(page, order);
> >> -               page_table_check_free(page, order);
> >> -               pgalloc_tag_sub(page, 1 << order);
> >> -
> >> -               /*
> >> -                * The page is isolated and accounted for.
> >> -                * Mark the codetag as empty to avoid accounting error
> >> -                * when the page is freed by unpoison_memory().
> >> -                */
> >> -               clear_page_tag_ref(page);
> >> -               return false;
> >> -       }
> >> -
> >>         VM_BUG_ON_PAGE(compound && compound_order(page) != order, page);
> >>
> >>         /*
> >> @@ -1395,8 +1381,26 @@ static __always_inline bool __free_pages_prepare(struct page *page,
> >>                                 }
> >>                         }
> >>                         tail_page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >> +                       hwpoisoned += PageHWPoison(tail_page);
> >>                 }
> >>         }
> >> +       hwpoisoned += PageHWPoison(page);
> >> +
> >> +       if (unlikely(hwpoisoned)) {
> >> +               /* Do not let hwpoison pages hit pcplists/buddy */
> >> +               reset_page_owner(page, order);
> >> +               page_table_check_free(page, order);
> >> +               pgalloc_tag_sub(page, 1 << order);
> >> +
> >> +               /*
> >> +                * The page is isolated and accounted for.
> >> +                * Mark the codetag as empty to avoid accounting error
> >> +                * when the page is freed by unpoison_memory().
> >> +                */
> >> +               clear_page_tag_ref(page);
> >> +               return false;
> >> +       }
> >> +
> >>         if (folio_test_anon(folio)) {
> >>                 mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1);
> >>                 folio->mapping = NULL;
> >> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> >>
> >>
> >> Of course, we could also fallback for order>0 to do some smart splitting.
> >
> > I think the freeing part is relatively fine, as tackled by this series.
>
> I think we should really do something like the above to detect hwpoison, to then
> fallback to a slow path.
>
> >
> >> Checking on allocation efficiently is indeed a bit more tricky. For compound
> >> pages prep_compound_page() will already walk all pages.
> >
> > Yeah and non-compound high-order pages should just go away eventually, so I
> > wouldn't worry about making them slower now.
> > But it's tricky because prep_compound_page() happens later when we don't
> > expect anything to fail anymore, so it would take some refactoring.
>
> Agreed.
>
> >
> >> Having hwpoison logic to just sync with the buddy when setting hwpoison flags
> >> would be nicer.
> >
> > I suggested that (soft-offline does it). There might be some downsides, but
> > I can't judge how much they would manifest in practice:
> >
> > https://lore.kernel.org/all/CACw3F50nVECJ%2Bk4g%3DQVZ80yzi_awCOcUiCuB0CYcUfxND9-7Tw@xxxxxxxxxxxxxx/
>
> Agreed.

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

I agree David's approach to check for PageHWPoison in
https://lore.kernel.org/linux-mm/8e48c288-2183-4d58-b69d-5f78344b5da5@xxxxxxxxxx
is much better than relying on PG_has_hwpoison flag. I can update this
in v7.

As for "should we split and salvage healthy subpages", I think we
should do it unless caller is FPI_NOLOCK, i.e. when freeing pages in
NMI / lockless context. The only caveat to call out is, if we reject
the entire folio for FPI_NOLOCK, the healthy sibling pages in that
folio are permanently lost from MemFree. So should we count them in
HardwareCorrupted then, as loss cascading to the corrupted page(s)?


>
> --
> Cheers,
>
> David