On 5/12/26 15:04, Breno Leitao wrote:
> On Tue, May 12, 2026 at 10:17:00AM +0200, David Hildenbrand (Arm) wrote:
>>> @@ -2348,6 +2348,7 @@ int memory_failure(unsigned long pfn, int flags)
>>> unsigned long page_flags;
>>> bool retry = true;
>>> int hugetlb = 0;
>>> + bool is_reserved;
>>>
>>> if (!sysctl_memory_failure_recovery)
>>> panic("Memory failure on page %lx", pfn);
>>> @@ -2411,6 +2412,18 @@ int memory_failure(unsigned long pfn, int flags)
>>> * In fact it's dangerous to directly bump up page count from 0,
>>> * that may make page_ref_freeze()/page_ref_unfreeze() mismatch.
>>> */
>>> + /*
>>> + * Pages with PG_reserved set are not currently managed by the
>>> + * page allocator (memblock-reserved memory, driver reservations,
>>> + * etc.), so classify them as kernel-owned for reporting.
>>> + *
>>> + * Sample the flag before get_hwpoison_page(): in the
>>> + * MF_COUNT_INCREASED path, get_any_page() can drop the caller's
>>> + * reference before returning -EIO, after which page->flags may
>>> + * have been reset by the allocator.
>>> + */
>>> + is_reserved = PageReserved(p);
>>> +
>>> res = get_hwpoison_page(p, flags);
>>> if (!res) {
>>> if (is_free_buddy_page(p)) {
>>> @@ -2432,7 +2445,11 @@ int memory_failure(unsigned long pfn, int flags)
>>> }
>>> goto unlock_mutex;
>>> } else if (res < 0) {
>>> - res = action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED);
>>> + if (is_reserved)
>>> + res = action_result(pfn, MF_MSG_KERNEL, MF_IGNORED);
>>> + else
>>> + res = action_result(pfn, MF_MSG_GET_HWPOISON,
>>> + MF_IGNORED);
>>> goto unlock_mutex;
>>> }
>>>
>>>
>>
>> It's a bit odd that we need this handling when we already have handling for
>> reserved pages in error_states[].
>>
>> HWPoisonHandlable() would always essentially reject PG_reserved pages. So
>> __get_hwpoison_page() ... would always fail? Making
>> get_hwpoison_page()->get_any_page() always fail?
>>
>> But then, we never call identify_page_state()? And never call me_kernel()?
>
> From what I read, it seems that error_states[0] = { reserved, reserved,
> MF_MSG_KERNEL, me_kernel }
> has been effectively dead code on the hwpoison-from-MCE path for a
> while.
>
> My v6 patch relabels the failure-path output to match what me_kernel() would
> have reported anyway.
>
>> This all looks very odd.
>>
>> Why would you even want to call get_hwpoison_page() in the first place if you
>> find PageReserved?
>
> Are you suggesting we should all the page action as soon as we detect the page
> is reserved and get out?
>
> Something as:
>
> if (PageReserved(p)) {
> res = action_result(pfn, MF_MSG_KERNEL, MF_IGNORED);
> goto unlock_mutex;
> }
>
> res = get_hwpoison_page(p, flags);
Or you combine this patch with the other patch and let simply
get_hwpoison_page() check that, and return an appropriate error code for
unhandable that you can process here?
Like, maybe, returning -EIO directly?
res = get_hwpoison_page(p, flags);
switch (res) {
case 0: /* Success */
...
break
case -EIO: /* Unhandable kernel page. */
...
break;
case -EBUSY: /* Race, try again? */
...
break;
case ...
}
You can add more return codes as you see fit.
--
Cheers,
David
