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 07242C433F5 for ; Fri, 15 Apr 2022 01:55:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 72CFC6B0071; Thu, 14 Apr 2022 21:55:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6DC936B0073; Thu, 14 Apr 2022 21:55:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5CB156B0074; Thu, 14 Apr 2022 21:55:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id 4F0576B0071 for ; Thu, 14 Apr 2022 21:55:20 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id 23D2863096 for ; Fri, 15 Apr 2022 01:55:20 +0000 (UTC) X-FDA: 79357446000.13.667DBA3 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf23.hostedemail.com (Postfix) with ESMTP id DB1F1140003 for ; Fri, 15 Apr 2022 01:55:18 +0000 (UTC) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.54]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4KffXF1F8Gz1HByR; Fri, 15 Apr 2022 09:54:37 +0800 (CST) Received: from [10.174.177.76] (10.174.177.76) by canpemm500002.china.huawei.com (7.192.104.244) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Fri, 15 Apr 2022 09:55:14 +0800 Subject: Re: [PATCH v8 1/3] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb() To: Mike Kravetz , Naoya Horiguchi , CC: Andrew Morton , Yang Shi , Dan Carpenter , Naoya Horiguchi , References: <20220408135323.1559401-1-naoya.horiguchi@linux.dev> <20220408135323.1559401-2-naoya.horiguchi@linux.dev> <5b665bcd-57f8-85ae-b0c4-c055875dbfff@oracle.com> From: Miaohe Lin Message-ID: <20e677e5-01aa-f8c0-0ce1-bf33da58b7ec@huawei.com> Date: Fri, 15 Apr 2022 09:55:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <5b665bcd-57f8-85ae-b0c4-c055875dbfff@oracle.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.177.76] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: DB1F1140003 X-Stat-Signature: cq1ih1xn3w6nzmonn495exmpdnreuopt Authentication-Results: imf23.hostedemail.com; dkim=none; spf=pass (imf23.hostedemail.com: domain of linmiaohe@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=linmiaohe@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com X-HE-Tag: 1649987718-55058 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: On 2022/4/15 1:56, Mike Kravetz wrote: > On 4/8/22 06:53, Naoya Horiguchi wrote: >> From: Naoya Horiguchi >> >> There is a race condition between memory_failure_hugetlb() and hugetlb >> free/demotion, which causes setting PageHWPoison flag on the wrong page. >> The one simple result is that wrong processes can be killed, but another >> (more serious) one is that the actual error is left unhandled, so no one >> prevents later access to it, and that might lead to more serious results >> like consuming corrupted data. >> >> Think about the below race window: >> >> CPU 1 CPU 2 >> memory_failure_hugetlb >> struct page *head = compound_head(p); >> hugetlb page might be freed to >> buddy, or even changed to another >> compound page. >> >> get_hwpoison_page -- page is not what we want now... >> >> The current code first does prechecks roughly and then reconfirms >> after taking refcount, but it's found that it makes code overly >> complicated, so move the prechecks in a single hugetlb_lock range. >> >> A newly introduced function, try_memory_failure_hugetlb(), always >> takes hugetlb_lock (even for non-hugetlb pages). That can be >> improved, but memory_failure() is rare in principle, so should >> not be a big problem. ... > > The above code works as designed, but may be a bit confusing. If HPageFreed() > we KNOW ref count is zero, so no need to even call get_page_unless_zero() as > it will always return false in this case. It might be more clear if written > as separate else if statements such as: > > } else if (HPageFreed(head)) { > ret = 0; > } else if (HPageMigratable(head)) { > ret = get_page_unless_zero(head); > if (ret) > count_increased = true; > This code here is consistent with the logic in get_hwpoison_huge_page. If change is required, they might need to be changed together. BTW: They look a bit confusing for me at first but I get used to it later. ;) Thanks! > Not insisting this be changed. Just easier to understand IMO. > > Again, thanks for your work on this! > > Reviewed-by: Mike Kravetz >