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 E95F7C433FE for ; Tue, 15 Feb 2022 09:28:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 529E36B0078; Tue, 15 Feb 2022 04:28:39 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D9C56B007B; Tue, 15 Feb 2022 04:28:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3C89B6B007D; Tue, 15 Feb 2022 04:28:39 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0110.hostedemail.com [216.40.44.110]) by kanga.kvack.org (Postfix) with ESMTP id 29F9D6B0078 for ; Tue, 15 Feb 2022 04:28:39 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id BF1B0181AC9C6 for ; Tue, 15 Feb 2022 09:28:38 +0000 (UTC) X-FDA: 79144489116.24.851E8ED Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf08.hostedemail.com (Postfix) with ESMTP id 57747160002 for ; Tue, 15 Feb 2022 09:28:36 +0000 (UTC) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4JybJD4cZRzZfGh; Tue, 15 Feb 2022 17:24:12 +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.2308.21; Tue, 15 Feb 2022 17:28:32 +0800 Subject: Re: [PATCH 4/8] mm/memory-failure.c: remove unneeded orig_head To: =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= CC: Naoya Horiguchi , "akpm@linux-foundation.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" References: <20220210141733.1908-1-linmiaohe@huawei.com> <20220210141733.1908-5-linmiaohe@huawei.com> <20220214145019.GD2624914@u2004> <20220215084813.GA1971011@hori.linux.bs1.fc.nec.co.jp> From: Miaohe Lin Message-ID: <9e6c4031-18eb-c4fb-d30c-3fa5767f07f8@huawei.com> Date: Tue, 15 Feb 2022 17:28:32 +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: <20220215084813.GA1971011@hori.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset="utf-8" Content-Language: en-US X-Originating-IP: [10.174.177.76] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 57747160002 X-Stat-Signature: 4f3kifz5qn8u66qcp1oxs3mfmeaai95b X-Rspam-User: Authentication-Results: imf08.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf08.hostedemail.com: domain of linmiaohe@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linmiaohe@huawei.com X-HE-Tag: 1644917316-601373 Content-Transfer-Encoding: quoted-printable 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/2/15 16:48, HORIGUCHI NAOYA(=E5=A0=80=E5=8F=A3 =E7=9B=B4=E4=B9=9F= ) wrote: > On Tue, Feb 15, 2022 at 11:14:07AM +0800, Miaohe Lin wrote: >> On 2022/2/14 22:50, Naoya Horiguchi wrote: >>> On Thu, Feb 10, 2022 at 10:17:29PM +0800, Miaohe Lin wrote: >>>> orig_head is used to check whether the page have changed compound pa= ges >>>> during the locking. But it's always equal to hpage. So we can use hp= age >>>> directly and remove this redundant one. >>>> >>>> Signed-off-by: Miaohe Lin >>>> --- >>>> mm/memory-failure.c | 5 ++--- >>>> 1 file changed, 2 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >>>> index 2dd7f35ee65a..4370c2f407c5 100644 >>>> --- a/mm/memory-failure.c >>>> +++ b/mm/memory-failure.c >>>> @@ -1691,7 +1691,6 @@ int memory_failure(unsigned long pfn, int flag= s) >>>> { >>>> struct page *p; >>>> struct page *hpage; >>>> - struct page *orig_head; >>>> struct dev_pagemap *pgmap; >>>> int res =3D 0; >>>> unsigned long page_flags; >>>> @@ -1737,7 +1736,7 @@ int memory_failure(unsigned long pfn, int flag= s) >>>> goto unlock_mutex; >>>> } >>>> >>>> - orig_head =3D hpage =3D compound_head(p); >>>> + hpage =3D compound_head(p); >>>> num_poisoned_pages_inc(); >>>> >>>> /* >>>> @@ -1821,7 +1820,7 @@ int memory_failure(unsigned long pfn, int flag= s) >>>> * The page could have changed compound pages during the locking. >>>> * If this happens just bail out. >>>> */ >>>> - if (PageCompound(p) && compound_head(p) !=3D orig_head) { >>>> + if (PageCompound(p) && compound_head(p) !=3D hpage) { >>> >>> I think that this if-check was intended to detect the case that page = p >>> belongs to a thp when memory_failure() is called and belongs to a com= pound >>> page in different size (like slab or some driver page) after the thp = is >>> split. But your suggestion makes me aware that the page p could be e= mbedded >>> on a thp again after thp split. I think this might be rare, but if i= t >> >> IIUC, this page can't be embedded on a thp again after thp split becau= se memory_failure hold >> an __extra__ page refcnt. I think there exist below race windows: >> >> memory_failure >> orig_head =3D hpage =3D compound_head(p); -- page is Non-compound ye= t >> < -- Page becomes compound page, like thp, slab, some driver page an= d even hugetlb page -- > >> get_hwpoison_page >> failed to split thp page, as hpage is Non-compound ... >> lock_page >> >>> happens the current if-check (or suggested one) cannot detect it. >>> So I feel that simply dropping compound_head() check might be better? >>> >>> - if (PageCompound(p) && compound_head(p) !=3D orig_head) { >>> + if (PageCompound(p)) { >> >> However this change could also catch the above race correctly. In fact= , we can't handle >> compound page here. But is it enough to just return -EBUSY here as it'= s really rare or >> we should do more things (like split thp, retry if in PageHuge case)? >=20 > Hmm, both could make sense and hard to judge to me, so it's upto you. > We already have goto label "try_again" so retrying might not be so surp= rising. >=20 try_again sounds a good idea. Will send a V2. Many thanks. > Thanks, > Naoya Horiguchi >=20