Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
fixup types are used to identify fixups which allow in kernel #MC
recovery, that is the Machine Check Safe Copy.
For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
and EX_TYPE_UACCESS when copy from user, and corrupted page is
isolated in this case, for MC-safe copy, memory_failure() is not
always called, some places, like __wp_page_copy_user, copy_subpage,
copy_user_gigantic_page and ksm_might_need_to_copy manually call
memory_failure_queue() to cope with such unhandled error pages,
recently coredump hwposion recovery support[1] is asked to do the
same thing, and there are some other already existed MC-safe copy
scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
exception, then kill_me_never() will be queued to call memory_failure()
in do_machine_check() to isolate corrupted page, which avoid calling
memory_failure_queue() after every MC-safe copy return.
[1] https://lkml.kernel.org/r/[email protected]
Signed-off-by: Kefeng Wang <[email protected]>
---
arch/x86/kernel/cpu/mce/severity.c | 3 +--
mm/ksm.c | 1 -
mm/memory.c | 12 +++---------
3 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
index c4477162c07d..63e94484c5d6 100644
--- a/arch/x86/kernel/cpu/mce/severity.c
+++ b/arch/x86/kernel/cpu/mce/severity.c
@@ -293,12 +293,11 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
case EX_TYPE_COPY:
if (!copy_user)
return IN_KERNEL;
- m->kflags |= MCE_IN_KERNEL_COPYIN;
fallthrough;
case EX_TYPE_FAULT_MCE_SAFE:
case EX_TYPE_DEFAULT_MCE_SAFE:
- m->kflags |= MCE_IN_KERNEL_RECOV;
+ m->kflags |= MCE_IN_KERNEL_RECOV | MCE_IN_KERNEL_COPYIN;
return IN_KERNEL_RECOV;
default:
diff --git a/mm/ksm.c b/mm/ksm.c
index 0156bded3a66..7abdf4892387 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2794,7 +2794,6 @@ struct page *ksm_might_need_to_copy(struct page *page,
if (new_page) {
if (copy_mc_user_highpage(new_page, page, address, vma)) {
put_page(new_page);
- memory_failure_queue(page_to_pfn(page), 0);
return ERR_PTR(-EHWPOISON);
}
SetPageDirty(new_page);
diff --git a/mm/memory.c b/mm/memory.c
index 5e2c6b1fc00e..c0f586257017 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2814,10 +2814,8 @@ static inline int __wp_page_copy_user(struct page *dst, struct page *src,
unsigned long addr = vmf->address;
if (likely(src)) {
- if (copy_mc_user_highpage(dst, src, addr, vma)) {
- memory_failure_queue(page_to_pfn(src), 0);
+ if (copy_mc_user_highpage(dst, src, addr, vma))
return -EHWPOISON;
- }
return 0;
}
@@ -5852,10 +5850,8 @@ static int copy_user_gigantic_page(struct folio *dst, struct folio *src,
cond_resched();
if (copy_mc_user_highpage(dst_page, src_page,
- addr + i*PAGE_SIZE, vma)) {
- memory_failure_queue(page_to_pfn(src_page), 0);
+ addr + i*PAGE_SIZE, vma))
return -EHWPOISON;
- }
}
return 0;
}
@@ -5871,10 +5867,8 @@ static int copy_subpage(unsigned long addr, int idx, void *arg)
struct copy_subpage_arg *copy_arg = arg;
if (copy_mc_user_highpage(copy_arg->dst + idx, copy_arg->src + idx,
- addr, copy_arg->vma)) {
- memory_failure_queue(page_to_pfn(copy_arg->src + idx), 0);
+ addr, copy_arg->vma))
return -EHWPOISON;
- }
return 0;
}
--
2.35.3
On Mon, May 08, 2023 at 10:22:33AM +0800, Kefeng Wang wrote:
> Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
> fixup types are used to identify fixups which allow in kernel #MC
> recovery, that is the Machine Check Safe Copy.
>
> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
> and EX_TYPE_UACCESS when copy from user, and corrupted page is
> isolated in this case, for MC-safe copy, memory_failure() is not
> always called, some places, like __wp_page_copy_user, copy_subpage,
> copy_user_gigantic_page and ksm_might_need_to_copy manually call
> memory_failure_queue() to cope with such unhandled error pages,
> recently coredump hwposion recovery support[1] is asked to do the
> same thing, and there are some other already existed MC-safe copy
> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>
> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
> exception, then kill_me_never() will be queued to call memory_failure()
> in do_machine_check() to isolate corrupted page, which avoid calling
> memory_failure_queue() after every MC-safe copy return.
>
> [1] https://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Kefeng Wang <[email protected]>
Looks good to me, thank you.
Reviewed-by: Naoya Horiguchi <[email protected]>
Hi Tony and all x86 maintainers, kindly ping, thanks.
On 2023/5/8 10:22, Kefeng Wang wrote:
> Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
> fixup types are used to identify fixups which allow in kernel #MC
> recovery, that is the Machine Check Safe Copy.
>
> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
> and EX_TYPE_UACCESS when copy from user, and corrupted page is
> isolated in this case, for MC-safe copy, memory_failure() is not
> always called, some places, like __wp_page_copy_user, copy_subpage,
> copy_user_gigantic_page and ksm_might_need_to_copy manually call
> memory_failure_queue() to cope with such unhandled error pages,
> recently coredump hwposion recovery support[1] is asked to do the
> same thing, and there are some other already existed MC-safe copy
> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>
> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
> exception, then kill_me_never() will be queued to call memory_failure()
> in do_machine_check() to isolate corrupted page, which avoid calling
> memory_failure_queue() after every MC-safe copy return.
>
> [1] https://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> arch/x86/kernel/cpu/mce/severity.c | 3 +--
> mm/ksm.c | 1 -
> mm/memory.c | 12 +++---------
> 3 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
> index c4477162c07d..63e94484c5d6 100644
> --- a/arch/x86/kernel/cpu/mce/severity.c
> +++ b/arch/x86/kernel/cpu/mce/severity.c
> @@ -293,12 +293,11 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
> case EX_TYPE_COPY:
> if (!copy_user)
> return IN_KERNEL;
> - m->kflags |= MCE_IN_KERNEL_COPYIN;
> fallthrough;
>
> case EX_TYPE_FAULT_MCE_SAFE:
> case EX_TYPE_DEFAULT_MCE_SAFE:
> - m->kflags |= MCE_IN_KERNEL_RECOV;
> + m->kflags |= MCE_IN_KERNEL_RECOV | MCE_IN_KERNEL_COPYIN;
> return IN_KERNEL_RECOV;
>
> default:
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 0156bded3a66..7abdf4892387 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2794,7 +2794,6 @@ struct page *ksm_might_need_to_copy(struct page *page,
> if (new_page) {
> if (copy_mc_user_highpage(new_page, page, address, vma)) {
> put_page(new_page);
> - memory_failure_queue(page_to_pfn(page), 0);
> return ERR_PTR(-EHWPOISON);
> }
> SetPageDirty(new_page);
> diff --git a/mm/memory.c b/mm/memory.c
> index 5e2c6b1fc00e..c0f586257017 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2814,10 +2814,8 @@ static inline int __wp_page_copy_user(struct page *dst, struct page *src,
> unsigned long addr = vmf->address;
>
> if (likely(src)) {
> - if (copy_mc_user_highpage(dst, src, addr, vma)) {
> - memory_failure_queue(page_to_pfn(src), 0);
> + if (copy_mc_user_highpage(dst, src, addr, vma))
> return -EHWPOISON;
> - }
> return 0;
> }
>
> @@ -5852,10 +5850,8 @@ static int copy_user_gigantic_page(struct folio *dst, struct folio *src,
>
> cond_resched();
> if (copy_mc_user_highpage(dst_page, src_page,
> - addr + i*PAGE_SIZE, vma)) {
> - memory_failure_queue(page_to_pfn(src_page), 0);
> + addr + i*PAGE_SIZE, vma))
> return -EHWPOISON;
> - }
> }
> return 0;
> }
> @@ -5871,10 +5867,8 @@ static int copy_subpage(unsigned long addr, int idx, void *arg)
> struct copy_subpage_arg *copy_arg = arg;
>
> if (copy_mc_user_highpage(copy_arg->dst + idx, copy_arg->src + idx,
> - addr, copy_arg->vma)) {
> - memory_failure_queue(page_to_pfn(copy_arg->src + idx), 0);
> + addr, copy_arg->vma))
> return -EHWPOISON;
> - }
> return 0;
> }
>
> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
> and EX_TYPE_UACCESS when copy from user, and corrupted page is
> isolated in this case, for MC-safe copy, memory_failure() is not
> always called, some places, like __wp_page_copy_user, copy_subpage,
> copy_user_gigantic_page and ksm_might_need_to_copy manually call
> memory_failure_queue() to cope with such unhandled error pages,
> recently coredump hwposion recovery support[1] is asked to do the
> same thing, and there are some other already existed MC-safe copy
> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>
> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
> exception, then kill_me_never() will be queued to call memory_failure()
> in do_machine_check() to isolate corrupted page, which avoid calling
> memory_failure_queue() after every MC-safe copy return.
>
> [1] https://lkml.kernel.org/r/[email protected]
Is this patch in addition to, or instead of, the earlier core dump patch?
I'd like to run some tests. Can you point me a the precise set of patches
that I should apply please?
-Tony
On 2023/5/20 0:17, Luck, Tony wrote:
>> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
>> and EX_TYPE_UACCESS when copy from user, and corrupted page is
>> isolated in this case, for MC-safe copy, memory_failure() is not
>> always called, some places, like __wp_page_copy_user, copy_subpage,
>> copy_user_gigantic_page and ksm_might_need_to_copy manually call
>> memory_failure_queue() to cope with such unhandled error pages,
>> recently coredump hwposion recovery support[1] is asked to do the
>> same thing, and there are some other already existed MC-safe copy
>> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>>
>> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
>> exception, then kill_me_never() will be queued to call memory_failure()
>> in do_machine_check() to isolate corrupted page, which avoid calling
>> memory_failure_queue() after every MC-safe copy return.
>>
>> [1] https://lkml.kernel.org/r/[email protected]
>
> Is this patch in addition to, or instead of, the earlier core dump patch?
This is an addition, in previous coredump patch, manually call
memory_failure_queue()
to be asked to cope with corrupted page, and it is similar to your
"Copy-on-write poison recovery"[1], but after some discussion, I think
we could add MCE_IN_KERNEL_COPYIN to all MC-safe copy, which will
cope with corrupted page in the core do_machine_check() instead of
do it one-by-one.
The related patch is
normal page CoW [1]
huge page CoW [2]
coredump [3]
ksm might copy [4]
[1] d302c2398ba2 ("mm, hwpoison: when copy-on-write hits poison, take
page offline")
a873dfe1032a ("mm, hwpoison: try to recover from copy-on write faults")
[2] 1cb9dc4b475c ("mm: hwpoison: support recovery from HugePage
copy-on-write faults")
[3] 245f09226893 ("mm: hwpoison: coredump: support recovery from
dump_user_range()")
[4] 6b970599e807 ("mm: hwpoison: support recovery from
ksm_might_need_to_copy()")
All of them are in v6.4-rc1.
Thanks.
Kefeng
>
> I'd like to run some tests. Can you point me a the precise set of patches
> that I should apply please?
>
> -Tony
>
>> Is this patch in addition to, or instead of, the earlier core dump patch?
>
> This is an addition, in previous coredump patch, manually call
> memory_failure_queue()
> to be asked to cope with corrupted page, and it is similar to your
> "Copy-on-write poison recovery"[1], but after some discussion, I think
> we could add MCE_IN_KERNEL_COPYIN to all MC-safe copy, which will
> cope with corrupted page in the core do_machine_check() instead of
> do it one-by-one.
Thanks for the context. I see how this all fits together now).
Your patch looks good.
Reviewed-by: Tony Luck <[email protected]>
-Tony
One small observation from testing. I injected to an application which consumed
the poisoned data and was sent a SIGBUS.
Kernel did not crash (hurrah!)
Console log said:
[ 417.610930] mce: [Hardware Error]: Machine check events logged
[ 417.618372] Memory failure: 0x89167f: recovery action for dirty LRU page: Recovered
... EDAC messages
[ 423.666918] MCE: Killing testprog:4770 due to hardware memory corruption fault at 7f8eccf35000
A core file was generated and saved in /var/lib/systemd/coredump
But my shell (/bin/bash) only said:
Bus error
not
Bus error (core dumped)
-Tony
On 2023/5/23 2:02, Luck, Tony wrote:
>>> Is this patch in addition to, or instead of, the earlier core dump patch?
>>
>> This is an addition, in previous coredump patch, manually call
>> memory_failure_queue()
>> to be asked to cope with corrupted page, and it is similar to your
>> "Copy-on-write poison recovery"[1], but after some discussion, I think
>> we could add MCE_IN_KERNEL_COPYIN to all MC-safe copy, which will
>> cope with corrupted page in the core do_machine_check() instead of
>> do it one-by-one.
>
> Thanks for the context. I see how this all fits together now).
>
> Your patch looks good.
>
> Reviewed-by: Tony Luck <[email protected]>
Thanks for your confirm.
>
> -Tony
>
> One small observation from testing. I injected to an application which consumed
> the poisoned data and was sent a SIGBUS.
>
> Kernel did not crash (hurrah!)
Yes, no crash is always great.
>
> Console log said:
>
> [ 417.610930] mce: [Hardware Error]: Machine check events logged
> [ 417.618372] Memory failure: 0x89167f: recovery action for dirty LRU page: Recovered
> ... EDAC messages
> [ 423.666918] MCE: Killing testprog:4770 due to hardware memory corruption fault at 7f8eccf35000
>
> A core file was generated and saved in /var/lib/systemd/coredump
>
> But my shell (/bin/bash) only said:
>
> Bus error
>
> not
>
> Bus error (core dumped)
No sure about the effect, but since there is kernel message and mcelog,
it seems that there is no big deal for the different :)
>
> -Tony
>
Hi x86/mm maintainers, could you pick this up as it has be reviewed
by Naoya and Tony, many thanks.
On 2023/5/8 10:22, Kefeng Wang wrote:
> Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
> fixup types are used to identify fixups which allow in kernel #MC
> recovery, that is the Machine Check Safe Copy.
>
> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
> and EX_TYPE_UACCESS when copy from user, and corrupted page is
> isolated in this case, for MC-safe copy, memory_failure() is not
> always called, some places, like __wp_page_copy_user, copy_subpage,
> copy_user_gigantic_page and ksm_might_need_to_copy manually call
> memory_failure_queue() to cope with such unhandled error pages,
> recently coredump hwposion recovery support[1] is asked to do the
> same thing, and there are some other already existed MC-safe copy
> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>
> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
> exception, then kill_me_never() will be queued to call memory_failure()
> in do_machine_check() to isolate corrupted page, which avoid calling
> memory_failure_queue() after every MC-safe copy return.
>
> [1] https://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Kefeng Wang <[email protected]>
On 5/7/23 19:22, Kefeng Wang wrote:
> Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
> fixup types are used to identify fixups which allow in kernel #MC
> recovery, that is the Machine Check Safe Copy.
>
> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
> and EX_TYPE_UACCESS when copy from user, and corrupted page is
> isolated in this case, for MC-safe copy, memory_failure() is not
> always called, some places, like __wp_page_copy_user, copy_subpage,
> copy_user_gigantic_page and ksm_might_need_to_copy manually call
> memory_failure_queue() to cope with such unhandled error pages,
> recently coredump hwposion recovery support[1] is asked to do the
> same thing, and there are some other already existed MC-safe copy
> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
That has to set some kind of record for run-on sentences. Could you
please try to rewrite this coherently?
> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
> exception, then kill_me_never() will be queued to call memory_failure()
> in do_machine_check() to isolate corrupted page, which avoid calling
> memory_failure_queue() after every MC-safe copy return.
Could you try to send a v2 of this with a clear problem statement?
What is the end user visible effect of the problem and of your solution?
On 2023/5/26 1:18, Dave Hansen wrote:
> On 5/7/23 19:22, Kefeng Wang wrote:
>> Both EX_TYPE_FAULT_MCE_SAFE and EX_TYPE_DEFAULT_MCE_SAFE exception
>> fixup types are used to identify fixups which allow in kernel #MC
>> recovery, that is the Machine Check Safe Copy.
>>
>> For now, the MCE_IN_KERNEL_COPYIN flag is only set for EX_TYPE_COPY
>> and EX_TYPE_UACCESS when copy from user, and corrupted page is
>> isolated in this case, for MC-safe copy, memory_failure() is not
>> always called, some places, like __wp_page_copy_user, copy_subpage,
>> copy_user_gigantic_page and ksm_might_need_to_copy manually call
>> memory_failure_queue() to cope with such unhandled error pages,
>> recently coredump hwposion recovery support[1] is asked to do the
>> same thing, and there are some other already existed MC-safe copy
>> scenarios, eg, nvdimm, dm-writecache, dax, which has similar issue.
>
> That has to set some kind of record for run-on sentences. Could you
> please try to rewrite this coherently?
>
>> The best way to fix them is set MCE_IN_KERNEL_COPYIN to MCE_SAFE
>> exception, then kill_me_never() will be queued to call memory_failure()
>> in do_machine_check() to isolate corrupted page, which avoid calling
>> memory_failure_queue() after every MC-safe copy return.
>
> Could you try to send a v2 of this with a clear problem statement?
>
:( will try to make it more clear.
> What is the end user visible effect of the problem and of your solution?
The corrupted page won't be isolated for MC-safe copy scenario, and it
could be accessed again by use application.