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 4F11DC4708D for ; Thu, 8 Dec 2022 02:45:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DFC058E0003; Wed, 7 Dec 2022 21:45:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DA9918E0001; Wed, 7 Dec 2022 21:45:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C97BD8E0003; Wed, 7 Dec 2022 21:45:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id BAA858E0001 for ; Wed, 7 Dec 2022 21:45:03 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B9222AB5AD for ; Thu, 8 Dec 2022 02:45:02 +0000 (UTC) X-FDA: 80217596844.24.B4AB031 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by imf26.hostedemail.com (Postfix) with ESMTP id B0C5B140008 for ; Thu, 8 Dec 2022 02:44:59 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf26.hostedemail.com: domain of lvying6@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=lvying6@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670467500; 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=MTgj/cToNHtFFu/UE92qP5wzPwr0A/ZlZXB2nWK4iVs=; b=5FpP1bWNCjqsWFaWgDzV6VA40ASIk/LuZaRUEmIh2QrSLmG2jDz1v3bRN6JsiRgIvS2c3K 1TpE0pc/DEWKrCOABDY0HWDdljMSqQ2/xfqEywJJ31kfGCOlqh7Rzj7w4GGXG3tDJxDE/F QEyl5Qe9YMRUd9ntHkaM8BNcnHpklGo= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf26.hostedemail.com: domain of lvying6@huawei.com designates 45.249.212.189 as permitted sender) smtp.mailfrom=lvying6@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670467500; a=rsa-sha256; cv=none; b=REon/qTjg1Pbm1G/JdHyrPeG13OO6S5wgHpRrT3XhyxDVPng/fuaqhAc+gjQFcLIu66qSo 8BESBdT6pRD66M/UveWslf8DkSb/OUGAEcDI3G9i4XW2Vjx+EHK6ez0GMxQYsjaV7mHZqn zWgu3S/u78wpZPZfN3N5Nx8V8DS683I= Received: from kwepemi500015.china.huawei.com (unknown [172.30.72.57]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4NSJLp5tzSzJpBF; Thu, 8 Dec 2022 10:41:22 +0800 (CST) Received: from [10.174.176.219] (10.174.176.219) by kwepemi500015.china.huawei.com (7.221.188.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Thu, 8 Dec 2022 10:44:53 +0800 Subject: Re: [RFC 2/2] ACPI: APEI: fix reboot caused by synchronous error loop because of memory_failure() failed To: Bixuan Cui , , , , , , , , , , CC: , , , , , , References: <20221205115111.131568-1-lvying6@huawei.com> <20221205115111.131568-3-lvying6@huawei.com> From: Lv Ying Message-ID: Date: Thu, 8 Dec 2022 10:44:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.176.219] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500015.china.huawei.com (7.221.188.92) X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: B0C5B140008 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: dkgn969mkjnmcwdb7nzz1wm5s56im16k X-HE-Tag: 1670467499-644529 X-HE-Meta: U2FsdGVkX1+vZWFQ9Wp1UspbT27dl1crXbcihbNznMVMUZQwEZ9N6nbHd/9AZS6S2A4vv07TABCpTDKnSXF5NFrxV1d4JMnwPBh6hexLd2optUQYutbTIN57u3+i/oOWXRxrytP+J5ckQhgXuEHDzsvE1IpdZcM9hkxvXJ+RzTufxIMP/J3BYEn9zYmajQ4HS77p52Ddd1+E78EVCxolgnRcTTOc8xYzcNjl7rDabPs+JcnF20ArKHc+aMWhXzuJ/VV7iYNXWMRUl9JJMm4aRjbHBcUnru0gX4+seTlsPJtJiLPORIn5sHB8OoEZP0ZpN7uqm6s9Kf/hTR4ajLRVxzk9PFZx31dhPHu9GlMsp39jA+/Zwfxu2WmWU4j4I9Jmk7lFgO1mPji4m2fP/Lm+4Brkw7i7PwnPRj7N6FmWY9HCVrv1zRsAWj7VHItXeabWWV6xGOq3p5KHttrUhxYh667Vl5JEdxr3ZA9Kfa2F+FGh9vsVHw0rBht0ygoUeDPBE7JNiyR8HsZVFzBdUIQcB2h9uXx4ze9k39KU9XpsQFBv+s1siKQEDx9h/0DOfo9Ow8ti8a4H4n73NpcFw93Q3BNmXoO4Ozmlv7H3AYMlbshfMB6mPb4AmOvzyQ9MmMcj8ynbCzYNiV0O0BKT2pB3TGJTqnFaXqTWtzG8edFNtiRyzowOhBWHZwEoUwyFvsjJvVzY4+tO6YI4tANoSYgrLLTzx9CfYK2zzXTmcrBRo0aNCdcjRtcCbv5h7WMkSIeP8100abIklfZBZT2xtNGBD2VvVawqNx3sdMK6W/vhPfS6MNdCl79OAxJ3okCw4PDAFaFXm53QAl2gCDrhryYZsRwKaTGqBBZIbEu3Jema21Q92OAMLFzq+g== 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: >> 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. > Hi Bixuan: Thanks for your review. I agree with you that ghes_proc_in_irq() is called in SDEI, SEA, NMI notify type, they are NMI-like notify, this function run some job which may not be NMI safe in IRQ context. And NMI may be asynchronous error. However, cureent kernel use ghes_kick_task_work in ghes_proc_in_irq(), there is an assumption here that ghes_proc_in_irq() are currently in the context of a synchronous exception, although this is not appropriate. The challenge for my patch is to prove the rationality of distinguishing synchronous errors. I do not have a good idea yet of distinguishing synchronous error by looking through ACPI/UEFI spec, so I sent this patchset for more input. And I resent RFC PATCH v1 [1]add this as TODO. > 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; > Thanks, there is indeed no need to pass 'sync' to __memory_failure_work_func(). MF_ACTION_REQUIRED can cover this, I will update it in the next version patchset. > 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 again, this patch is based on synchronous error is not distinguished from asynchronous error, in that case, kill_accessing_process() run in kthread worker may not kill current thread. Now, based on the first patch, this SEA loop can be handled. But this patch is also needed reference x86 kill_me_maybe(), I update this patch in RFC PATCH v1[1]. I will integrate this patch into the first patch, because this patch commit message is not suitable based on the first patch. [1] https://lore.kernel.org/linux-mm/20221207093935.1972530-1-lvying6@huawei.com/T/ -- Thanks! Lv Ying