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

[Vlastimil Babka (SUSE)]

On 7/27/26 16:20, David Hildenbrand (Arm) wrote:
> On 7/22/26 10:27, Vlastimil Babka (SUSE) wrote:
>> On 7/17/26 15:06, William Roche wrote:
>>> On 7/17/26 12:18, David Hildenbrand (Arm) wrote:
>>>>
>>>> I still don't like the complexity of this, in particular, as we have different
>>>> mechanisms in the page allocator already to try handling this,
>>>>
>>>> We also do have cases where we set the hwpoison bit, while a page is just about
>>>> to get allocated from the buddy. So before we take it off the buddy, we might
>>>> just hand out the page.
>>>>
>>>> check_new_pages() seems to check for PageHWPoison() and make us not hand out
>>>> such pages. It's guarded by "check_pages" but it seems to do exactly what we are
>>>> looking for, now?
>>>
>>>
>>> Just adding a comment about this aspect:
>>> The check_new_pages() mechanism used by the __rmqueue functions should
>>> filter these pages out, but this has been disabled by default in 2023
>>> with:
>>> [PATCH] mm, page_alloc: reduce page alloc/free sanity checks
>>> https://lore.kernel.org/all/20230216095131.17336-1-vbabka@xxxxxxx
>>>
>>> So it would need to be enabled back, taking some of the performance hit.
>>> (and I personally think that it has to be done)
>> 
>> Would it truly fix the issue, or rather there would still be a race window
>> left where we check that there's no hwpoison flag in the re-enabled check,
>> and only then someone sets it?
> 
> Why are we checking PageHWPoison at all then in check_new_page()?

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.

> I think we created a mess.

Yes, the PageHWPoison handling was made part of debugging sanity check and
then not recognized properly as load-bearing later.

> The PageHWPoison check is not just a "nice to have" sanity check for kernel bugs.
> 
> So it should never have been optimized out that way before reworking the bigger
> picture.

Sorry!

>> 
>> Also, can the hardware actually detect a problem with a page that nobody
>> accesses? I guess if yes, it's only in some corner cases.
> 
> Yes, quite frequently I think.
> 
>> 
>> So I'm wary about penalizing the allocator paths again.
> 
> I get the feeling that we don't have a proper plan on how to handle HWPoisoned
> pages. We should take a step back and discuss how we actually want to handle
> them instead of optimizing here and there and creating more of a mess.

Fully agree on having a proper plan, because I got the feeling it's been a
whack-a-mole approach for years. If we then decide to put the check back
(and live with the residual race window), it should be handled completely
separately from check_new_page().