Re: [PATCH v5 1/4] mm/page_alloc: only free healthy pages in high-order has_hwpoisoned folio

[Zi Yan]

On Mon Jun 22, 2026 at 11:20 AM EDT, Zi Yan wrote:
> On Mon Jun 22, 2026 at 4:35 AM EDT, Vlastimil Babka (SUSE) wrote:
>> On 6/22/26 03:13, Jiaqi Yan wrote:
>>> On Thu, Jun 18, 2026 at 9:04 AM Zi Yan <ziy@xxxxxxxxxx> wrote:
>>>> >> when copy fails with EHWPOISON), the next list_add() is going to cause
>>>> >
>>>> > I think we should fix compaction_free() (as a separate patch preceding this
>>>> > one) to not proceed if prepare returns false. It could in theory already
>>>> > happen before this patch due to a random memory corruption of struct page
>>>> > causing some of the existing checks to fail.
>>>>
>>>> Something like below should do the job, plus
>>>> Fixes: 733aea0b3a7bb ("mm/compaction: add support for >0 order folio memory compaction.")
>>>>
>>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>>> index b776f35ad0200..b2104cbe63477 100644
>>>> --- a/mm/compaction.c
>>>> +++ b/mm/compaction.c
>>>> @@ -1876,10 +1876,12 @@ static void compaction_free(struct folio *dst, unsigned long data)
>>>>         struct page *page = &dst->page;
>>>>
>>>>         if (folio_put_testzero(dst)) {
>>>> -               free_pages_prepare(page, order);
>>>> +               if (!free_pages_prepare(page, order))
>>> 
>>> ... for other cases that free_pages_prepare() fails, this is
>>> absolutely necessary for correctness. Do you mind sending it out for
>>> formal review (I will be happy to help review).
>>
>> I think it would be faster if you just added it to your series with a
>> Suggested-by: or something? Unless Zi objects. The change is straightforward.
>
> Let me send it as a hotfix separately, since Andrew prefers to have a
> standalone hotfix to avoid splitting a series.
>
>>
>> I'd perhaps do just a
>>
>> if (folio_put_testzero(dst) && free_pages_prepare(page, order))
>>
>> to avoid the goto?
>
> Will do.
>>
>>>> +                       goto skip;
>>>>                 list_add(&dst->lru, &cc->freepages[order]);
>>>>                 cc->nr_freepages += 1 << order;
>>>>         }
>>>> +skip:
>>>>         cc->nr_migratepages += 1 << order;
>>>>         /*
>>>>          * someone else has referenced the page, we cannot take it back to our
>>
>> Maybe in the comment here add also the possibility that _prepare failed().
>
> Sure.

Done.

Link: https://lore.kernel.org/linux-mm/20260622-handle_free_pages_prepare_in_compaction_free-v1-1-fcf3b14abcf7@xxxxxxxxxx/



-- 
Best Regards,
Yan, Zi