Re: [PATCH v3] x86/mce: Set PG_hwpoison page flag to avoid the capture kernel panic

[Zhiquan Li]

On 2023/10/14 13:12, Luck, Tony wrote:
>> Co-developed-by: Youquan Song <youquan.song@xxxxxxxxx>
>> Signed-off-by: Youquan Song <youquan.song@xxxxxxxxx>
>> Signed-off-by: Zhiquan Li <zhiquan1.li@xxxxxxxxx>
>> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
>> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
>> Reviewed-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
>> Cc: Borislav Petkov <bp@xxxxxxxxx>
>> Link: https://lore.kernel.org/all/20230719211625.298785-1-tony.luck@xxxxxxxxx/#t
> 
> This still has problems.  You should have removed my Signed-off-by. You should NOT
> have added Ingo's Signed-off-by (the only time to add someone else's Signed-off-by
> is when paired with a Co-developed-by).
> 

Oh, this is V3, not RESEND, I should reset the SoB chain.
Thanks for your reminder, Tony.

If my understanding is correct, the version below fixes the tag list.

Best Regards,
Zhiquan

========>
From: Zhiquan Li <zhiquan1.li@xxxxxxxxx>
Date: Sat, 14 Oct 2023 13:03:17 -0600
Subject: [PATCH v3] x86/mce: Set PG_hwpoison page flag to avoid the capture kernel panic

Memory errors don't happen very often, especially the severity is fatal.
However, in large-scale scenarios, such as data centers, it might still
happen.  For some MCE fatal error cases, the kernel might call
mce_panic() to terminate the production kernel directly, thus there is
no opportunity to queue a task for calling memory_failure() which will
try to make the kernel survive via memory failure handling.

Unfortunately, the capture kernel will panic for the same reason if it
touches the error memory again.  The consequence is that only an
incomplete vmcore is left for sustaining engineers, it's a big headache
for them to make clear what happened in the past.

The main task of kdump kernel is providing a "window" - /proc/vmcore,
for the dump program to access old memory.  A dump program running in
userspace determines the "policy".  Which pages need to be dumped is
determined by the configuration of dump program, it reads out the pages
that the sustaining engineer is interested in and excludes the rest.

Likewise, the dump program can exclude the poisoned page to avoid
touching the error page again, the prerequisite is the PG_hwpoison page
flag is set correctly by kernel.  The de facto dump program
(makedumpfile) already supports this function in a decade ago.  To set
the PG_hwpoison flag in the production kernel just before it panics is
the only missing step to make everything work.

And it would not introduce additional overhead in capture kernel or
conflict with other hwpoision-related code in production kernel.  It
leverages the already existing mechanisms to fix the issue as much as
possible, so the code changes are lightweight.

Co-developed-by: Youquan Song <youquan.song@xxxxxxxxx>
Signed-off-by: Youquan Song <youquan.song@xxxxxxxxx>
Signed-off-by: Zhiquan Li <zhiquan1.li@xxxxxxxxx>
Reviewed-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>

---

V2: https://lore.kernel.org/all/20230914030539.1622477-1-zhiquan1.li@xxxxxxxxx/

Changes since V2:
- Rebased to v6.6-rc5.
- Explained full scenario in commit message per Boris's suggestion.
- Included Ingo's fixes.
  Link: https://lore.kernel.org/all/ZRsUpM%2FXtPAE50Rm@xxxxxxxxx/

V1: https://lore.kernel.org/all/20230127015030.30074-1-tony.luck@xxxxxxxxx/

Changes since V1:
- Revised the commit message as per Naoya's suggestion.
- Replaced "TODO" comment in code with comments based on mailing list
  discussion on the lack of value in covering other page types.
- Added the tag from Naoya.
  Link: https://lore.kernel.org/all/20230327083739.GA956278@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
---
 arch/x86/kernel/cpu/mce/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 6f35f724cc14..905e80c776b8 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -233,6 +233,7 @@ static noinstr void mce_panic(const char *msg, struct mce *final, char *exp)
 	struct llist_node *pending;
 	struct mce_evt_llist *l;
 	int apei_err = 0;
+	struct page *p;
 
 	/*
 	 * Allow instrumentation around external facilities usage. Not that it
@@ -286,6 +287,17 @@ static noinstr void mce_panic(const char *msg, struct mce *final, char *exp)
 	if (!fake_panic) {
 		if (panic_timeout == 0)
 			panic_timeout = mca_cfg.panic_timeout;
+		/*
+		 * Kdump can exclude the HWPoison page to avoid touching the error
+		 * page again, the prerequisite is that the PG_hwpoison page flag is
+		 * set.  However, for some MCE fatal error cases, there is no
+		 * opportunity to queue a task for calling memory_failure(), and as a
+		 * result, the capture kernel panics.  So mark the page as HWPoison
+		 * before kernel panic() for MCE.
+		 */
+		p = pfn_to_online_page(final->addr >> PAGE_SHIFT);
+		if (final && (final->status & MCI_STATUS_ADDRV) && p)
+			SetPageHWPoison(p);
 		panic(msg);
 	} else
 		pr_emerg(HW_ERR "Fake kernel panic: %s\n", msg);
-- 
2.25.1