在 2025/2/12 16:09, Miaohe Lin 写道:
On 2025/2/11 14:02, Shuai Xue wrote:
When an uncorrected memory error is consumed there is a race between
the CMCI from the memory controller reporting an uncorrected error
with a UCNA signature, and the core reporting and SRAR signature
machine check when the data is about to be consumed.
If the CMCI wins that race, the page is marked poisoned when
uc_decode_notifier() calls memory_failure(). For dirty pages,
memory_failure() invokes try_to_unmap() with the TTU_HWPOISON flag,
converting the PTE to a hwpoison entry. However, for clean pages, the
TTU_HWPOISON flag is cleared, leaving the PTE unchanged and not converted
to a hwpoison entry. Consequently, for an unmapped dirty page, the PTE is
marked as a hwpoison entry allowing kill_accessing_process() to:
- call walk_page_range() and return 1
- call kill_proc() to make sure a SIGBUS is sent
- return -EHWPOISON to indicate that SIGBUS is already sent to the process
and kill_me_maybe() doesn't have to send it again.
Conversely, for clean pages where PTE entries are not marked as hwpoison,
kill_accessing_process() returns -EFAULT, causing kill_me_maybe() to send a
SIGBUS.
Console log looks like this:
Memory failure: 0x827ca68: corrupted page was clean: dropped without side effects
Memory failure: 0x827ca68: recovery action for clean LRU page: Recovered
Memory failure: 0x827ca68: already hardware poisoned
mce: Memory error not recovered
To fix it, return -EHWPOISON if no hwpoison PTE entry is found, preventing
an unnecessary SIGBUS.
Thanks for your patch.
Fixes: 046545a661af ("mm/hwpoison: fix error page recovered but reported "not recovered"")
Signed-off-by: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx>
---
mm/memory-failure.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 995a15eb67e2..f9a6b136a6f0 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -883,10 +883,9 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
(void *)&priv);
if (ret == 1 && priv.tk.addr)
kill_proc(&priv.tk, pfn, flags);
- else
- ret = 0;
mmap_read_unlock(p->mm);
- return ret > 0 ? -EHWPOISON : -EFAULT;
+
+ return ret >= 0 ? -EHWPOISON : -EFAULT;
IIUC, kill_accessing_process() is supposed to return -EHWPOISON to notify that SIGBUS is already
sent to the process and kill_me_maybe() doesn't have to send it again. But with your change,
kill_accessing_process() will return -EHWPOISON even if SIGBUS is not sent. Does this break
the semantics of -EHWPOISON?
Yes, from the comment of kill_me_maybe(),
* -EHWPOISON from memory_failure() means that it already sent SIGBUS
* to the current process with the proper error info,
* -EOPNOTSUPP means hwpoison_filter() filtered the error event,
this patch break the comment.
But the defination of EHWPOISON is quite different from the comment.
#define EHWPOISON 133 /* Memory page has hardware error */
As for this issue, returning 0 or EHWPOISON can both prevent a SIGBUS signal
from being sent in kill_me_maybe().
Which way do you prefer?
BTW I scanned the code of walk_page_range(). It seems with implementation of hwpoison_walk_ops
walk_page_range() will only return 0 or 1, i.e. always >= 0. So kill_accessing_process() will always
return -EHWPOISON if this patch is applied.
Correct me if I miss something.
Yes, you are right. Let's count the cases one by one:
1. clean page: try_to_remap(!TTU_HWPOISON), walk_page_range() will return 0 and
we should not send sigbus in kill_me_maybe().
2. dirty page:
2.1 MCE wins race
CMCI:w/o Action Require MCE: w/ Action Require
TestSetPageHWPoison
TestSetPageHWPoison
return -EHWPOISON
try_to_unmap(TTU_HWPOISON)
kill_proc in hwpoison_user_mappings()
If MCE wins the race, because the flag of memory_fialure() called by CMCI is
not set as MF_ACTION_REQUIRED, everything goes well, kill_proc() will send
SIGBUS in hwpoison_user_mappings().
2.2 CMCI win
CMCI:w/o Action Require MCE: w/ Action Require
TestSetPageHWPoison
try_to_unmap(TTU_HWPOISON)
walk_page_range() return 1 due to hwpoison PTE entry
kill_proc in kill_accessing_process()
If the CMCI wins the race, we need to kill the process in
kill_accessing_process(). And if try_to_remap() success, everything goes well,
kill_proc() will send SIGBUS in kill_accessing_process().
But if try_to_remap() fails, the PTE entry will not be marked as hwpoison, and
walk_page_range() return 0 as case 1 clean page, NO SIGBUS will be sent.
In summary, hwpoison_walk_ops cannot distinguish between try_to_unmap failing
and causing the PTE entry not to be set to hwpoison, and a clean page that
originally does not have the PTE entry set to hwpoison.
+naoya for orginal patch intend.
Thanks.
Best Regard,
Shuai
