From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 742F1C77B6E for ; Wed, 12 Apr 2023 04:05:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D4C08900002; Wed, 12 Apr 2023 00:05:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CD5566B0075; Wed, 12 Apr 2023 00:05:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B751A900002; Wed, 12 Apr 2023 00:05:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 8A1256B0074 for ; Wed, 12 Apr 2023 00:05:15 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2513980DDC for ; Wed, 12 Apr 2023 04:05:15 +0000 (UTC) X-FDA: 80671398990.27.C6CC308 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by imf27.hostedemail.com (Postfix) with ESMTP id 71EAA4000D for ; Wed, 12 Apr 2023 04:05:12 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf27.hostedemail.com: domain of tanxiaofei@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=tanxiaofei@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681272313; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=38pdj5Z2LJE8WWGslznEd4ciSV2FH6rPlARRajfQ4yc=; b=PL5vF9nrxrTLxCudJUj2nZ1m71yU5q/UcCSPnOKzr55pNcb48gN/9v9QWz40XUMLNeYaSe IhdEYq8d75jdKaeORCBLkrz3CMe+MRZI2JpeuTtHt93NoBT4J2JtTGAs/+FR89NUupWtOr SQHOItoKB2KaB6OMsShoQ4ePMLnlTBQ= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf27.hostedemail.com: domain of tanxiaofei@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=tanxiaofei@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681272313; a=rsa-sha256; cv=none; b=jZvgJ51o0/lSRCyM6HvdMfJqCqxdJAnH2tVPmK6pCKDQ3bROIVyKFGHlu0iS3rj6KZQIzk 5LYsgerFUEz0bjZsxlY3eji9P1vdIcl1XZW8CxvtP+Z0sBWsO4AKktFxePe07/GHwfpJUy YoGR1Aa9ebLegz3rbR5pvD9Tn9pXiWE= Received: from dggpemm500001.china.huawei.com (unknown [172.30.72.53]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4Px8Gv5qTCzDskf; Wed, 12 Apr 2023 12:04:23 +0800 (CST) Received: from [10.67.100.236] (10.67.100.236) by dggpemm500001.china.huawei.com (7.185.36.107) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Wed, 12 Apr 2023 12:05:07 +0800 Message-ID: Date: Wed, 12 Apr 2023 12:05:07 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Subject: Re: [PATCH v5 2/2] ACPI: APEI: handle synchronous exceptions in task work To: Shuai Xue , , , CC: , , , , , , , , , , , , , , , , , , References: <20221027042445.60108-1-xueshuai@linux.alibaba.com> <20230411104842.37079-3-xueshuai@linux.alibaba.com> From: Xiaofei Tan In-Reply-To: <20230411104842.37079-3-xueshuai@linux.alibaba.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.100.236] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemm500001.china.huawei.com (7.185.36.107) X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: 71EAA4000D X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: 1owu1qnx98xo1g9ixpag1xpq517ch5co X-HE-Tag: 1681272312-380361 X-HE-Meta: U2FsdGVkX1+zf+Ai8ufdW5CR+YIUnbdjo9hGPvca8nFca51RDpYCbkKuhK0n/S7i2ljlWQSmR5Zdf/WlSw85wot1ciZQD02spzQqtHxHrb+ohSTmipyygzFLytd2GyCLc6hAjN53cLFXKwwhOtrHOEpxdQSo1yojoYomqK0M772xu5yKKplswGr57aTF7Syxv/LU+F/bIupsiIiuLDABuy+Ldv2sdx8rFaC2MesnqZKL+gWWHeb2V8ZIEedh3aJLyqcAVeIRWElqlnYaIGROOuL8gvarQwqXqqK1ffdCOz7NhmYrPdauczTftStuSQXM26k29g3g7VYZbstCxj83HvrLngbZGQAsE9CDBm3M2UvKJqrmv7+5iDUFIEmkKGJunMketZ1bs5+s7b5ARJPgJDkJxn2mMXZd+Tk5kMObDyw/i2jtB1xhFXQyU09j0U9ae1/Ug/6sG9zs43Zfjo23e849ypFZmApSzbjFgxqBpl58ZV/J0ISdXbENkU7vWSvyFRGMrANOjaqszEdQYU2pzJBTB5Otu6U0xTgj3VHXDaKU8AoFiTMF5D5Mcr4QA1LY289LMKoNuklG4ciVhQLe0Vic8DWG26oPMLalHb6QTSAK1on4ZcrhRoh3maj8mTSp2ySGfsgOjIkbPx0vZotIDNZvNsE3wMgofOHuJnZNF0eLf1vDco04WcTgFmWsXO6D1sPy5/fx6RoQ+nEo3L+rihVQN0Zaqdr2DIR+44XUjIBRfYsb68yigUJW7bB3Oal8l9QtQifesTklNyQwGkhkioUsv+li1AqonEwRjK9STD77cjPvM7+ldFQz5DEObfUF18d9SQJ+F80M/Rifr3Mos1NRettQwpo2xl7eLGPVi2kVttZnQC//Wj0T639ivjyIQUCzZGkptjCJQwk0HIh+oIaaNqcXUH5XVmupIkrs8BmnvtCo1IPJ/ldGB4dRsjUN9v265fnLXRXzpxmMtP+ 7bSZ2e4c 00P71XxVwVEZj1G5wIulywLqzX7JAYIIkAiKT4hX+fYFw6N3PWj18Be393vsV8oDT8itzOkNcV32ZXVXSIpBSTuR76FNRBlXc+r4O+aIYg70/Nd1A64Ts3xI9Hv2cHM6oCUV3evuH42gJYMKAC4/ibRnH6TErl3MwqnYuSeAEbUFT3SEpYVwb0qA70NWV0yVZKoZ3 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 在 2023/4/11 18:48, Shuai Xue 写道: > Hardware errors could be signaled by synchronous interrupt, e.g. when an > error is detected by a background scrubber, or signaled by synchronous > exception, e.g. when an uncorrected error is consumed. Both synchronous and > asynchronous error are queued and handled by a dedicated kthread in > workqueue. > > 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 workqueue so that the > work for synchronous error is processed before returning to user-space. > The trick ensures that the corrupted page is unmapped and poisoned. And > after returning to user-space, the task starts at current instruction which > triggering a page fault in which kernel will send SIGBUS to current process > due to VM_FAULT_HWPOISON. > > However, the memory failure recovery for hwpoison-aware mechanisms does not > work as expected. For example, hwpoison-aware user-space processes like > QEMU register their customized SIGBUS handler and enable early kill mode by > seting PF_MCE_EARLY at initialization. Then the kernel will directy notify > the process by sending a SIGBUS signal in memory failure with wrong > si_code: the actual user-space process accessing the corrupt memory > location, but its memory failure work is handled in a kthread context, so > it will send SIGBUS with BUS_MCEERR_AO si_code to the actual user-space > process instead of BUS_MCEERR_AR in kill_proc(). > > To this end, separate synchronous and asynchronous error handling into > different paths like X86 platform does: > > - valid synchronous errors: queue a task_work to synchronously send SIGBUS > before ret_to_user. > - valid asynchronous errors: queue a work into workqueue to asynchronously > handle memory failure. > - abnormal branches such as invalid PA, unexpected severity, no memory > failure config support, invalid GUID section, OOM, etc. > > Then for valid synchronous errors, the current context in memory failure is > exactly belongs to the task consuming poison data and it will send SIBBUS > with proper si_code. > > Fixes: 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for synchronous errors") > Signed-off-by: Shuai Xue > Tested-by: Ma Wupeng > --- > drivers/acpi/apei/ghes.c | 91 +++++++++++++++++++++++++++------------- > include/acpi/ghes.h | 3 -- > mm/memory-failure.c | 13 ------ > 3 files changed, 61 insertions(+), 46 deletions(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index c479b85899f5..4b70955e25f9 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -452,28 +452,51 @@ static void ghes_clear_estatus(struct ghes *ghes, > } > > /* > - * Called as task_work before returning to user-space. > - * Ensure any queued work has been done before we return to the context that > - * triggered the notification. > + * struct sync_task_work - for synchronous RAS event > + * > + * @twork: callback_head for task work > + * @pfn: page frame number of corrupted page > + * @flags: fine tune action taken > + * > + * Structure to pass task work to be handled before > + * ret_to_user via task_work_add(). > */ > -static void ghes_kick_task_work(struct callback_head *head) > +struct sync_task_work { > + struct callback_head twork; > + u64 pfn; > + int flags; > +}; > + > +static void memory_failure_cb(struct callback_head *twork) > { > - struct acpi_hest_generic_status *estatus; > - struct ghes_estatus_node *estatus_node; > - u32 node_len; > + int ret; > + struct sync_task_work *twcb = > + container_of(twork, struct sync_task_work, twork); > > - estatus_node = container_of(head, struct ghes_estatus_node, task_work); > - if (IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE)) > - memory_failure_queue_kick(estatus_node->task_work_cpu); > + ret = memory_failure(twcb->pfn, twcb->flags); > + kfree(twcb); > > - estatus = GHES_ESTATUS_FROM_NODE(estatus_node); > - node_len = GHES_ESTATUS_NODE_LEN(cper_estatus_len(estatus)); > - gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node, node_len); > + if (!ret) > + return; > + > + /* > + * -EHWPOISON from memory_failure() means that it already sent SIGBUS > + * to the current process with the proper error info, > + * -EOPNOTSUPP means hwpoison_filter() filtered the error event, > + * > + * In both cases, no further processing is required. > + */ > + if (ret == -EHWPOISON || ret == -EOPNOTSUPP) > + return; > + > + pr_err("Memory error not recovered"); The print could add the following SIGBUS signal sending. Such as "Sending SIGBUS to current task due to memory error not recovered" > + force_sig(SIGBUS); > } > > static bool ghes_do_memory_failure(u64 physical_addr, int flags) > { > unsigned long pfn; > + struct sync_task_work *twcb; > > if (!IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE)) > return false; > @@ -486,6 +509,18 @@ static bool ghes_do_memory_failure(u64 physical_addr, int flags) > return false; > } > > + if (flags == MF_ACTION_REQUIRED && current->mm) { > + twcb = kmalloc(sizeof(*twcb), GFP_ATOMIC); > + if (!twcb) > + return false; > + > + twcb->pfn = pfn; > + twcb->flags = flags; > + init_task_work(&twcb->twork, memory_failure_cb); > + task_work_add(current, &twcb->twork, TWA_RESUME); > + return true; > + } > + > memory_failure_queue(pfn, flags); > return true; > } > @@ -1000,9 +1035,8 @@ static void ghes_proc_in_irq(struct irq_work *irq_work) > struct ghes_estatus_node *estatus_node; > struct acpi_hest_generic *generic; > struct acpi_hest_generic_status *estatus; > - bool task_work_pending; > + bool queued, sync; > u32 len, node_len; > - int ret; > > llnode = llist_del_all(&ghes_estatus_llist); > /* > @@ -1015,27 +1049,25 @@ static void ghes_proc_in_irq(struct irq_work *irq_work) > estatus_node = llist_entry(llnode, struct ghes_estatus_node, > llnode); > estatus = GHES_ESTATUS_FROM_NODE(estatus_node); > + sync = is_hest_sync_notify(estatus_node->ghes); > len = cper_estatus_len(estatus); > node_len = GHES_ESTATUS_NODE_LEN(len); > - task_work_pending = ghes_do_proc(estatus_node->ghes, estatus); > + > + queued = ghes_do_proc(estatus_node->ghes, estatus); > + /* > + * If no memory failure work is queued for abnormal synchronous > + * errors, do a force kill. > + */ > + if (sync && !queued) > + force_sig(SIGBUS); Could also add one similar print here as above Apart from this, Reviewed-by: Xiaofei Tan > + > if (!ghes_estatus_cached(estatus)) { > generic = estatus_node->generic; > if (ghes_print_estatus(NULL, generic, estatus)) > ghes_estatus_cache_add(generic, estatus); > } > - > - 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, > - TWA_RESUME); > - if (ret) > - estatus_node->task_work.func = NULL; > - } > - > - if (!estatus_node->task_work.func) > - gen_pool_free(ghes_estatus_pool, > - (unsigned long)estatus_node, node_len); > + gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node, > + node_len); > > llnode = next; > } > @@ -1096,7 +1128,6 @@ static int ghes_in_nmi_queue_one_entry(struct ghes *ghes, > > estatus_node->ghes = ghes; > estatus_node->generic = ghes->generic; > - estatus_node->task_work.func = NULL; > estatus = GHES_ESTATUS_FROM_NODE(estatus_node); > > if (__ghes_read_estatus(estatus, buf_paddr, fixmap_idx, len)) { > diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h > index 3c8bba9f1114..e5e0c308d27f 100644 > --- a/include/acpi/ghes.h > +++ b/include/acpi/ghes.h > @@ -35,9 +35,6 @@ struct ghes_estatus_node { > struct llist_node llnode; > struct acpi_hest_generic *generic; > struct ghes *ghes; > - > - int task_work_cpu; > - struct callback_head task_work; > }; > > struct ghes_estatus_cache { > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index fae9baf3be16..6ea8c325acb3 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2355,19 +2355,6 @@ static void memory_failure_work_func(struct work_struct *work) > } > } > > -/* > - * Process memory_failure work queued on the specified CPU. > - * Used to avoid return-to-userspace racing with the memory_failure workqueue. > - */ > -void memory_failure_queue_kick(int cpu) > -{ > - struct memory_failure_cpu *mf_cpu; > - > - mf_cpu = &per_cpu(memory_failure_cpu, cpu); > - cancel_work_sync(&mf_cpu->work); > - memory_failure_work_func(&mf_cpu->work); > -} > - > static int __init memory_failure_init(void) > { > struct memory_failure_cpu *mf_cpu;