Linux Documentation
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Lance Yang <lance.yang@linux.dev>
Cc: linmiaohe@huawei.com, akpm@linux-foundation.org,
	david@kernel.org,  ljs@kernel.org, vbabka@kernel.org,
	rppt@kernel.org, surenb@google.com,  mhocko@suse.com,
	shuah@kernel.org, nao.horiguchi@gmail.com, rostedt@goodmis.org,
	 mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
	corbet@lwn.net,  skhan@linuxfoundation.org, liam@infradead.org,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org,
	 linux-trace-kernel@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH v7 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
Date: Thu, 14 May 2026 07:37:14 -0700	[thread overview]
Message-ID: <agXcPleVC9LGVCmj@gmail.com> (raw)
In-Reply-To: <20260514132830.25622-1-lance.yang@linux.dev>

On Thu, May 14, 2026 at 09:28:30PM +0800, Lance Yang wrote:
> 
> On Wed, May 13, 2026 at 08:39:33AM -0700, Breno Leitao wrote:
> >get_any_page() collapses three different failure modes into a single
> >-EIO return:
> >
> >  * the put_page race in the !count_increased path;
> >  * the HWPoisonHandlable() rejection that bounces out of
> >    __get_hwpoison_page() with -EBUSY and exhausts shake_page() retries;
> >  * the HWPoisonHandlable() rejection that goes through the
> >    count_increased / put_page / shake_page retry loop.
> >
> >The first is transient (the page is racing with the allocator).  The
> >second can be either transient (a userspace folio briefly off LRU
> >during migration/compaction) or stable (slab/vmalloc/page-table/
> >kernel-stack pages).  The third describes a stable kernel-owned page
> >that the count_increased=true caller already held a reference on.
> >
> >Distinguish them on the return path: keep -EIO for both the put_page
> >race and the -EBUSY-after-retries branch (shake_page() cannot drag a
> >folio back from active migration, so we cannot prove the page is
> >permanently kernel-owned from there), keep -EBUSY for the allocation
> >race (unchanged), and return -ENOTRECOVERABLE only from the
> >count_increased-true HWPoisonHandlable() rejection that exhausts its
> >retries -- the caller's reference is structural evidence that the
> >page is owned by the kernel.
> >
> >Extend the unhandlable-page pr_err() to fire for either errno and
> >update the get_hwpoison_page() kerneldoc.
> >
> >memory_failure() still folds every negative return into
> >MF_MSG_GET_HWPOISON via its existing "else if (res < 0)" branch, so
> >this patch is a no-op for users of memory_failure() and only changes
> >the errno that soft_offline_page() can propagate to its callers.  A
> >follow-up wires the new return code through memory_failure() and
> >reports MF_MSG_KERNEL for the unrecoverable cases.
> >
> >Suggested-by: David Hildenbrand <david@kernel.org>
> >Signed-off-by: Breno Leitao <leitao@debian.org>
> >---
> > mm/memory-failure.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> >diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >index 49bcfbd04d213..bae883df3ccb2 100644
> >--- a/mm/memory-failure.c
> >+++ b/mm/memory-failure.c
> >@@ -1408,6 +1408,15 @@ static int get_any_page(struct page *p, unsigned long flags)
> > 				shake_page(p);
> > 				goto try_again;
> > 			}
> >+			/*
> >+			 * Return -EIO rather than -ENOTRECOVERABLE: this
> >+			 * branch is also reached for pages that are merely
> >+			 * off-LRU transiently (e.g. a folio in the middle
> >+			 * of migration or compaction), which shake_page()
> >+			 * cannot drag back.  The caller cannot prove the
> >+			 * page is permanently kernel-owned from here, so
> >+			 * keep it on the recoverable errno.
> >+			 */
> > 			ret = -EIO;
> > 			goto out;
> > 		}
> >@@ -1427,10 +1436,10 @@ static int get_any_page(struct page *p, unsigned long flags)
> > 			goto try_again;
> > 		}
> > 		put_page(p);
> >-		ret = -EIO;
> >+		ret = -ENOTRECOVERABLE;
> > 	}
> > out:
> >-	if (ret == -EIO)
> >+	if (ret == -EIO || ret == -ENOTRECOVERABLE)
> > 		pr_err("%#lx: unhandlable page.\n", page_to_pfn(p));
> > 
> > 	return ret;
> >@@ -1487,7 +1496,10 @@ static int __get_unpoison_page(struct page *page)
> >  *         -EIO for pages on which we can not handle memory errors,
> >  *         -EBUSY when get_hwpoison_page() has raced with page lifecycle
> >  *         operations like allocation and free,
> >- *         -EHWPOISON when the page is hwpoisoned and taken off from buddy.
> >+ *         -EHWPOISON when the page is hwpoisoned and taken off from buddy,
> >+ *         -ENOTRECOVERABLE for stable kernel-owned pages the handler
> >+ *         cannot recover (PG_reserved, slab, vmalloc, page tables,
> >+ *         kernel stacks, and similar non-LRU/non-buddy pages).
> 
> Did you test this patch series? I don't see how we ever get to
> -ENOTRECOVERABLE there ...

Yes, I did. I am using the following test case:

https://github.com/leitao/linux/commit/cfebe84ddeab5ac34ed456331db980d57e7025dc

	# RUN_DESTRUCTIVE=1 tools/testing/selftests/mm/hwpoison-panic.sh
	# enabling /proc/sys/vm/panic_on_unrecoverable_memory_failure
	# injecting hwpoison at phys 0x2a00000 (Kernel rodata)
	# expecting kernel panic: 'Memory failure: <pfn>: unrecoverable page'
	[  501.113256] Memory failure: 0x2a00: recovery action for reserved kernel page: Ignored
	[  501.113956] Kernel panic - not syncing: Memory failure: 0x2a00: unrecoverable page


> Even with MF_COUNT_INCREASED, the first pass does:
> 
> 	if (flags & MF_COUNT_INCREASED)
> 		count_increased = true;
> 
> 	[...]
> 
> 	if (PageHuge(p) || HWPoisonHandlable(p, flags)) {
> 		ret = 1;
> 	} else {
> 		if (pass++ < GET_PAGE_MAX_RETRY_NUM) { <-
> 			put_page(p);
> 			shake_page(p);
> 			count_increased = false;
> 			goto try_again; <-
> 		}
> 		put_page(p);
> 		ret = -ENOTRECOVERABLE;
> 	}
> 
> Then we come back with count_increased=false:
> 
> try_again:
> 	if (!count_increased) {
> 		ret = __get_hwpoison_page(p, flags); <-
> 		if (!ret) {
> 		[...]
> 		} else if (ret == -EBUSY) { <-
> 		[...]
> 			ret = -EIO;
> 			goto out; <-
> 		}
> 	}
> 
> For slab/vmalloc/page-table pages, __get_hwpoison_page() returns -EBUSY:
> 
> 	if (!HWPoisonHandlable(&folio->page, flags))
> 		return -EBUSY;
> 
> so they still seem to end up as -EIO ... Am I missing something?

You are not, and thanks for catching this. I traced it again and the
-ENOTRECOVERABLE branch is unreachable for slab/vmalloc/page-table pages
exactly as you described. The __get_hwpoison_page() → -EBUSY → shake → retry
loop catches them first and they exit as -EIO.

The selftest I am using (link above) only validated the PageReserved
short-circuit added in patch 3, which lives in memory_failure() and never
reaches get_any_page().

I even thought about this code path, and I was not convinced we should return
-ENOTRECOVERABLE, thus I documented the following (as in this current patch)

	@@ -1408,6 +1408,15 @@ static int get_any_page(struct page *p, unsigned long flags)
			shake_page(p);
			goto try_again;
		}
	+            /*
	+             * Return -EIO rather than -ENOTRECOVERABLE: this
	+             * branch is also reached for pages that are merely
	+             * off-LRU transiently (e.g. a folio in the middle
	+             * of migration or compaction), which shake_page()
	+             * cannot drag back.  The caller cannot prove the
	+             * page is permanently kernel-owned from here, so
	+             * keep it on the recoverable errno.
	+             */
		ret = -EIO;

  reply	other threads:[~2026-05-14 14:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 15:39 [PATCH v7 0/6] mm/memory-failure: add panic option for unrecoverable pages Breno Leitao
2026-05-13 15:39 ` [PATCH v7 1/6] mm/memory-failure: drop dead error_states[] entry for reserved pages Breno Leitao
2026-05-13 20:10   ` David Hildenbrand (Arm)
2026-05-14 10:55     ` Breno Leitao
2026-05-14  9:12   ` Lance Yang
2026-05-13 15:39 ` [PATCH v7 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE Breno Leitao
2026-05-14 13:28   ` Lance Yang
2026-05-14 14:37     ` Breno Leitao [this message]
2026-05-13 15:39 ` [PATCH v7 3/6] mm/memory-failure: report MF_MSG_KERNEL for unrecoverable kernel pages Breno Leitao
2026-05-13 15:39 ` [PATCH v7 4/6] mm/memory-failure: short-circuit PG_reserved before get_hwpoison_page() Breno Leitao
2026-05-13 19:49   ` David Hildenbrand (Arm)
2026-05-14 11:06     ` Breno Leitao
2026-05-13 15:39 ` [PATCH v7 5/6] mm/memory-failure: add panic option for unrecoverable pages Breno Leitao
2026-05-13 15:39 ` [PATCH v7 6/6] Documentation: document panic_on_unrecoverable_memory_failure sysctl Breno Leitao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agXcPleVC9LGVCmj@gmail.com \
    --to=leitao@debian.org \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linmiaohe@huawei.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=ljs@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mhocko@suse.com \
    --cc=nao.horiguchi@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=rppt@kernel.org \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox