| Message ID | 20230508022233.13890-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mce: set MCE_IN_KERNEL_COPYIN for all MC-Safe Copy | expand |
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/20230417045323.11054-1-wangkefeng.wang@huawei.com > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Looks good to me, thank you. Reviewed-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
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/20230417045323.11054-1-wangkefeng.wang@huawei.com > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > 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/20230417045323.11054-1-wangkefeng.wang@huawei.com 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/20230417045323.11054-1-wangkefeng.wang@huawei.com > > 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 <tony.luck@intel.com> -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 <tony.luck@intel.com> 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/20230417045323.11054-1-wangkefeng.wang@huawei.com > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
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.
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; }
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/20230417045323.11054-1-wangkefeng.wang@huawei.com Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/x86/kernel/cpu/mce/severity.c | 3 +-- mm/ksm.c | 1 - mm/memory.c | 12 +++--------- 3 files changed, 4 insertions(+), 12 deletions(-)