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 C1BD4C43334 for ; Tue, 28 Jun 2022 03:01:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4C26A8E0002; Mon, 27 Jun 2022 23:01:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 472468E0001; Mon, 27 Jun 2022 23:01:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 33A098E0002; Mon, 27 Jun 2022 23:01:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 208C38E0001 for ; Mon, 27 Jun 2022 23:01:21 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id D5919C64 for ; Tue, 28 Jun 2022 03:01:20 +0000 (UTC) X-FDA: 79626143520.28.3AE0B3C Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by imf09.hostedemail.com (Postfix) with ESMTP id EAC5514002E for ; Tue, 28 Jun 2022 03:01:17 +0000 (UTC) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4LX8Sq1HCczkWJn; Tue, 28 Jun 2022 10:59:23 +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; Tue, 28 Jun 2022 11:01:12 +0800 Subject: Re: [PATCH v2 1/9] mm/hugetlb: remove checking hstate_is_gigantic() in return_unused_surplus_pages() To: Mike Kravetz CC: Muchun Song , Naoya Horiguchi , Andrew Morton , David Hildenbrand , Liu Shixin , Yang Shi , Oscar Salvador , "linux-kernel@vger.kernel.org" , Linux-MM , =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= References: <20220623235153.2623702-1-naoya.horiguchi@linux.dev> <20220623235153.2623702-2-naoya.horiguchi@linux.dev> <0b69e3ef-0123-4575-b68d-4d9b2067aa0e@huawei.com> <20220624083428.GA2070418@hori.linux.bs1.fc.nec.co.jp> <20220627060231.GA2159330@hori.linux.bs1.fc.nec.co.jp> From: Miaohe Lin Message-ID: <227ccd06-a945-e4c5-8cff-37817f777781@huawei.com> Date: Tue, 28 Jun 2022 11:01:11 +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: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit 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 ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656385279; a=rsa-sha256; cv=none; b=UOzzAejGzqkxt6c6v0ePMJ7+nkoDFzUcutf+ddl+VgsS51eUU3/YE2B1wKm0opePDd+SfZ MxIo7Ffq6I0duy8qkR0QN3kj4Ksmnz4kvt9w6AXRM3IeCp2ZOAcPUiCpdyLjDCUN/rAi4Q torCCXE/8dKO0jPT2GErZEZkq7TNZbs= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf09.hostedemail.com: domain of linmiaohe@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=linmiaohe@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656385279; 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=JQy4U8JSZNfcUZJ5SIhxEqnS66Dohr8XDwp1xZZK/I0=; b=oIs10aJbeQ0aCQbC+JC4p7IS1JNqY0NhEUe1Ch+rdKGvXg/f4+Ei5yBNr8He2zxEQDwRgc a4rz0frjHN05hfm/zqHI9aUXwMZ9NwcautBkinWKQR6XjNJfrXZ2l3lp64dXTdd23nAKpl +VN1b0IZYzUET0WtyFt+QujMqX1EEWI= X-Rspam-User: X-Stat-Signature: weo5gpjr3f9cp9yd4aarsabkge9u4af8 X-Rspamd-Queue-Id: EAC5514002E Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf09.hostedemail.com: domain of linmiaohe@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=linmiaohe@huawei.com X-Rspamd-Server: rspam03 X-HE-Tag: 1656385277-93575 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/6/28 1:25, Mike Kravetz wrote: > On 06/27/22 06:02, HORIGUCHI NAOYA(堀口 直也) wrote: >> On Fri, Jun 24, 2022 at 12:11:07PM -0700, Mike Kravetz wrote: >>> On 06/24/22 08:34, HORIGUCHI NAOYA(堀口 直也) wrote: >>>> On Fri, Jun 24, 2022 at 04:15:19PM +0800, Miaohe Lin wrote: >>>>> On 2022/6/24 16:03, Muchun Song wrote: >>>>>> On Fri, Jun 24, 2022 at 10:25:48AM +0800, Miaohe Lin wrote: >>>>>>> On 2022/6/24 7:51, Naoya Horiguchi wrote: >>>>>>>> From: Naoya Horiguchi >>>>>>> >>>>>>> IIUC it might be better to do the below check: >>>>>>> /* >>>>>>> * Cannot return gigantic pages currently if runtime gigantic page >>>>>>> * allocation is not supported. >>>>>>> */ >>>>>>> if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported()) >>>>>>> goto out; >>>>>>> >>>>>> >>>>>> The change looks good to me. However, the comments above is unnecessary >>>>>> since gigantic_page_runtime_supported() is straightforward. >>>>> >>>>> Agree. The comments can be removed. >>>> >>>> Thank you, both. Adding !gigantic_page_runtime_supported without comment >>>> makes sense to me. >>> >>> The change above makes sense to me. However, ... >>> >>> If we make the change above, will we have the same strange situation described >>> in the commit message when !gigantic_page_runtime_supported() is true? >>> >>> IIUC, !gigantic_page_runtime_supported implies that gigantic hugetlb >>> pages can not be allocated or freed at run time. They can only be >>> allocated at boot time. So, there should NEVER be surplus gigantic >>> pages if !gigantic_page_runtime_supported(). >> >> I have the same understanding as the above. >> >>> To avoid this situation, >>> perhaps we should change set_max_huge_pages as follows (not tested)? >> >> The suggested diff looks clearer about what it does, so I'd like to take it >> in the next version. Then, what do we do on the "if (hstate_if_gigantic())" >> check in return_unused_surplus_pages in the original suggestion? Should it >> be kept as is, or removed, or checked with !gigantic_page_runtime_supported()? >> I guess that the new checks prevent calling return_unused_surplus_pages() >> during pool shrinking, so the check seems not necessary any more. > > My first thought was to keep the check in return_unused_surplus_pages() as it > is called in other code paths. However, it SHOULD only try to free surplus > hugetlb pages. With the modifications to set_max_huge_pages we will not > have any surplus gigantic pages if !gigantic_page_runtime_supported, so > the check can be removed. > > Also note that we never try to dynamically allocate surplus gigantic pages. > This also is left over from the time when we could not easily allocate a > gigantic page at runtime. It would not surprise me if someone found a use > case to ease this restriction in the future. Especially so if 1G THP support > is ever added. If this happens, the check would be necessary and I would > guess that it would not be added. > > Sorry for thinking our loud!!! Although not necessary, it 'might' be a good > idea to leave the check because it would be overlooked if dynamic allocation > of gigantic surplus pages is ever added. I do not have a strong opinion. > > P.S. This also reminds me that a similar check should be added to the > demote hugetlb code path. It would be bad if !gigantic_page_runtime_supported > and we demoted a gigantic page into numerous non-gigantic pages. I will > send a patch. Out-of-topic There're some places check "if (hstate_is_gigantic(h))" while others check "if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())". Should we make it explicit in some manner when gigantic_page_runtime_supported is needed to make code easier to follow? Just a trivial suggestion. Thanks! >