On Tue, Mar 09, 2021 at 02:35:34PM +0800, Aili Yao wrote:
> When the page is already poisoned, another memory_failure() call in the
> same page now return 0, meaning OK. For nested memory mce handling, this
> behavior may lead to mce looping, Example:
>
> 1.When LCME is enabled, and there are two processes A && B running on
> different core X && Y separately, which will access one same page, then
> the page corrupted when process A access it, a MCE will be rasied to
> core X and the error process is just underway.
>
> 2.Then B access the page and trigger another MCE to core Y, it will also
> do error process, it will see TestSetPageHWPoison be true, and 0 is
> returned.
>
> 3.The kill_me_maybe will check the return:
>
> 1244 static void kill_me_maybe(struct callback_head *cb)
> 1245 {
>
> 1254 if (!memory_failure(p->mce_addr >> PAGE_SHIFT, flags) &&
> 1255 !(p->mce_kflags & MCE_IN_KERNEL_COPYIN)) {
> 1256 set_mce_nospec(p->mce_addr >> PAGE_SHIFT,
> p->mce_whole_page);
> 1257 sync_core();
> 1258 return;
> 1259 }
>
> 1267 }
>
> 4. The error process for B will end, and may nothing happened if
> kill-early is not set, The process B will re-excute instruction and get
> into mce again and then loop happens. And also the set_mce_nospec()
> here is not proper, may refer to commit fd0e786d9d09 ("x86/mm,
> mm/hwpoison: Don't unconditionally unmap kernel 1:1 pages").
>
> For other cases which care the return value of memory_failure() should
> check why they want to process a memory error which have already been
> processed. This behavior seems reasonable.
Other reviewers shared ideas about the returned value, but actually
I'm not sure which the best one is (EBUSY is not that bad).
What we need to fix the reported issue is to return non-zero value
for "already poisoned" case (the value itself is not so important).
Other callers of memory_failure() (mostly test programs) could see
the change of return value, but they can already see EBUSY now and
anyway they should check dmesg for more detail about why failed,
so the impact of the change is not so big.
>
> Signed-off-by: Aili Yao <yaoaili@xxxxxxxxxxxx>
Reviewed-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
Thanks,
Naoya Horiguchi
