On 5/12/26 19:58, [email protected] wrote: > > > On 5/12/2026 1:17 AM, David Hildenbrand (Arm) wrote: >> On 5/11/26 17:38, Breno Leitao wrote: >>> When get_hwpoison_page() returns a negative value, distinguish >>> reserved pages from other failure cases by reporting MF_MSG_KERNEL >>> instead of MF_MSG_GET_HWPOISON. Reserved pages belong to the kernel >>> and should be classified accordingly for proper handling. >>> >>> Sample PG_reserved before the get_hwpoison_page() call. In the >>> MF_COUNT_INCREASED path get_any_page() can drop the caller's >>> reference before returning -EIO, after which the underlying page may >>> have been freed and reallocated with page->flags reset; reading >>> PageReserved(p) at that point would observe stale or unrelated state. >>> The pre-call snapshot reflects what the page actually was at the >>> time of the failure event. >>> >>> Acked-by: Miaohe Lin <[email protected]> >>> Reviewed-by: Lance Yang <[email protected]> >>> Signed-off-by: Breno Leitao <[email protected]> >>> --- >>> mm/memory-failure.c | 19 ++++++++++++++++++- >>> 1 file changed, 18 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >>> index 866c4428ac7ef..f112fb27a8ff6 100644 >>> --- a/mm/memory-failure.c >>> +++ b/mm/memory-failure.c >>> @@ -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()? >> >> This all looks very odd. >> >> Why would you even want to call get_hwpoison_page() in the first place if you >> find PageReserved? >> > > Ah, good point! > It seems to me that all unhandable pages should head out to > identify_page_state: > > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2411,6 +2411,10 @@ 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. > */ > + > + if (!HWPoisonHandlable(page, flags) > + goto identify_page_state; > + > res = get_hwpoison_page(p, flags); > if (!res) { > if (is_free_buddy_page(p)) {
That's one option, or we just let get_hwpoison_page() return clearer error codes, let it take care of checking PageReserved, and process the error codes return by get_hwpoison_page() in a better way. -- Cheers, David
