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

[Vlastimil Babka (SUSE)]

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.

I'd perhaps do just a

if (folio_put_testzero(dst) && free_pages_prepare(page, order))

to avoid the goto?

>> +                       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().

>>
>> >
>> >> trouble. Or you can rename the function to
>> >> __free_pages_prepare_and_free_has_hwpoison()? At least, caller knows the
>> >> potential side effect.
>> >
>> > Uh that's long. All callers are from PAGE ALLOCATOR mm-subsystem, it's not a
>> > driver API so we'll know what we're doing (famous last words :)
>>
>> The name above is a half joke. :)
>>
>> BTW, I do not even trust myself sometimes. ;) Just look at the
>> compaction_free() issue I introduced myself. But I do not want to be too
>> pedantic here. So a comment above __free_pages_prepare() should be
>> sufficient.
> 
> There is already a comment about the possible freeing case.
> 
>>
>> --
>> Best Regards,
>> Yan, Zi
>>