| Message ID | 20220916050535.26625-1-xueshuai@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | ACPI: APEI: do not add task_work for outside context error | expand |
在 2022/9/16 PM1:05, Shuai Xue 写道: > If an error is detected as a result of user-space process accessing a > corrupt memory location, the CPU may take an abort. Then the platform > firmware reports kernel via NMI like notifications, e.g. NOTIFY_SEA, > NOTIFY_SOFTWARE_DELEGATED, etc. > > For NMI like notifications, commit 7f17b4a121d0 ("ACPI: APEI: Kick the > memory_failure() queue for synchronous errors") keep track of whether > memory_failure() work was queued, and make task_work pending to flush out > the queue so that the work is processed before return to user-space. > > The code use init_mm to check whether the error occurs in user space: > > if (current->mm != &init_mm) > > The condition is always true, becase _nobody_ ever has "init_mm" as a real > VM any more (Sorry, I forgot to describe the side effect.) If an error is detected outside of the current execution context (e.g. when detected by a background scrubber), the current could be any thread. When a kernel thread is interrupted, the work ghes_kick_task_work deferred to task_work will never be processed because entry_handler returns to call ret_to_kernel() instead of ret_to_user(). Consequently, the estatus_node alloced from ghes_estatus_pool in ghes_in_nmi_queue_one_entry will not be released. After around 200 allocations in our platform, the ghes_estatus_pool will run of memory and ghes_in_nmi_queue_one_entry returns ENOMEM. As a result, the event failed to be processed. sdei: event 805 on CPU 113 failed with error: -2 Finally, a lot of unhandled events may cause platform firmware to exceed some threshold and reboot. Best Regards, Shuai and should generally just do > > if (current->mm) > > as described in active_mm.rst documentation. > > Then if an error is detected outside of the current execution context (e.g. > when detected by a background scrubber), do not add task_work as the > original patch intends to do. > > Fixes: 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for synchronous errors") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > --- > drivers/acpi/apei/ghes.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index d91ad378c00d..80ad530583c9 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -985,7 +985,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work) > ghes_estatus_cache_add(generic, estatus); > } > > - if (task_work_pending && current->mm != &init_mm) { > + if (task_work_pending && current->mm) { > estatus_node->task_work.func = ghes_kick_task_work; > estatus_node->task_work_cpu = smp_processor_id(); > ret = task_work_add(current, &estatus_node->task_work,
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index d91ad378c00d..80ad530583c9 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -985,7 +985,7 @@ static void ghes_proc_in_irq(struct irq_work *irq_work) ghes_estatus_cache_add(generic, estatus); } - if (task_work_pending && current->mm != &init_mm) { + if (task_work_pending && current->mm) { estatus_node->task_work.func = ghes_kick_task_work; estatus_node->task_work_cpu = smp_processor_id(); ret = task_work_add(current, &estatus_node->task_work,
If an error is detected as a result of user-space process accessing a corrupt memory location, the CPU may take an abort. Then the platform firmware reports kernel via NMI like notifications, e.g. NOTIFY_SEA, NOTIFY_SOFTWARE_DELEGATED, etc. For NMI like notifications, commit 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for synchronous errors") keep track of whether memory_failure() work was queued, and make task_work pending to flush out the queue so that the work is processed before return to user-space. The code use init_mm to check whether the error occurs in user space: if (current->mm != &init_mm) The condition is always true, becase _nobody_ ever has "init_mm" as a real VM any more and should generally just do if (current->mm) as described in active_mm.rst documentation. Then if an error is detected outside of the current execution context (e.g. when detected by a background scrubber), do not add task_work as the original patch intends to do. Fixes: 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for synchronous errors") Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> --- drivers/acpi/apei/ghes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)