[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ddc81946-8b76-ea49-ebf5-f2de2e30540d@linux.alibaba.com>
Date: Wed, 7 Dec 2022 20:29:04 +0800
From: Bixuan Cui <cuibixuan@...ux.alibaba.com>
To: Lv Ying <lvying6@...wei.com>, rafael@...nel.org, lenb@...nel.org,
james.morse@....com, tony.luck@...el.com, bp@...en8.de,
naoya.horiguchi@....com, linmiaohe@...wei.com,
akpm@...ux-foundation.org, xueshuai@...ux.alibaba.com,
ashish.kalra@....com
Cc: xiezhipeng1@...wei.com, wangkefeng.wang@...wei.com,
xiexiuqi@...wei.com, tanxiaofei@...wei.com,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [RFC 2/2] ACPI: APEI: fix reboot caused by synchronous error loop
because of memory_failure() failed
在 2022/12/5 19:51, Lv Ying 写道:
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 3b6ac3694b8d..4c1c558f7161 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2266,7 +2266,11 @@ static void __memory_failure_work_func(struct work_struct *work, bool sync)
> break;
> if (entry.flags & MF_SOFT_OFFLINE)
> soft_offline_page(entry.pfn, entry.flags);
> - else if (!sync || (entry.flags & MF_ACTION_REQUIRED))
> + else if (sync) {
> + if ((entry.flags & MF_ACTION_REQUIRED) &&
> + memory_failure(entry.pfn, entry.flags))
> + force_sig_mceerr(BUS_MCEERR_AR, 0, 0);
> + } else
> memory_failure(entry.pfn, entry.flags);
Hi,
Some of the ideas in this patch are wrong :-(
1. As Shuai Xue said, it is wrong to judge synchronization error and
asynchronization error through functions such as
memory_failure_queue_kick()/ghes_proc()/ghes_proc_in_irq(), because both
synchronization error and asynchronization error may go to the same
notification.
2. There is no need to pass 'sync' to __memory_failure_work_func(),
because memory_failure() can directly handle synchronous and
asynchronous errors according to entry.flags & MF_ACTION_REQUIRED:
entry.flags & MF_ACTION_REQUIRED == 1: Action: poison page and kill task
for synchronous error
entry.flags & MF_ACTION_REQUIRED == 0: Action: poison page for
asynchronous error
Reference x86:
do_machine_check # MCE, synchronous
->kill_me_maybe
->memory_failure(p->mce_addr >> PAGE_SHIFT, MF_ACTION_REQUIRED);
uc_decode_notifier # CMCI, asynchronous
->memory_failure(pfn, 0)
At the same time, the modification here is repeated with your patch 01
if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
- flags = 0;
+ flags = sync ? MF_ACTION_REQUIRED : 0;
3. Why add 'force_sig_mceerr(BUS_MCEERR_AR, 0, 0)' after
memory_failure(pfn, MF_ACTION_REQUIRED)?
The task will be killed in memory_failure():
if poisoned, kill_accessing_process()->kill_proc()
if not poisoned, hwpoison_user_mappings()->collect_procs()->kill_procs()
Reference x86 to handle synchronous error:
kill_me_maybe()
{
int flags = MF_ACTION_REQUIRED;
ret = memory_failure(p->mce_addr >> PAGE_SHIFT, flags);
if (!ret) {
...
return;
}
if (ret == -EHWPOISON || ret == -EOPNOTSUPP)
return;
pr_err("Memory error not recovered");
kill_me_now(cb);
}
Thanks,
Bixuan Cui
Powered by blists - more mailing lists