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 77E48E7AD57 for ; Tue, 3 Oct 2023 14:38:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E78148D007C; Tue, 3 Oct 2023 10:38:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E28068D0003; Tue, 3 Oct 2023 10:38:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CEF328D007C; Tue, 3 Oct 2023 10:38:22 -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 C01878D0003 for ; Tue, 3 Oct 2023 10:38:22 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 911FB140380 for ; Tue, 3 Oct 2023 14:38:22 +0000 (UTC) X-FDA: 81304405644.05.6EB6439 Received: from out-199.mta0.migadu.com (out-199.mta0.migadu.com [91.218.175.199]) by imf09.hostedemail.com (Postfix) with ESMTP id 910E1140030 for ; Tue, 3 Oct 2023 14:38:20 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=ci+JFf9B; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf09.hostedemail.com: domain of yajun.deng@linux.dev designates 91.218.175.199 as permitted sender) smtp.mailfrom=yajun.deng@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696343900; 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:dkim-signature; bh=NtOgS8kdqdev2oPEOGg7PvbjyZjOnfyVKUNFOralQmQ=; b=r8KlDxSK5+3GyC+FUrYORTrW9M/HLHmgHLz5UjJdr/WHNbVMJIUDB6eAjSdh3Ip0EgZQ3c G0SWLLwT8debSny2VxB3iB7akWv0/VrZe2JEfh72inSr5SVL9BLRKP67aOYbEvLOQMfAPv fpjy+rHiHmndIVkOqnaO4PMHpYhXoKQ= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=ci+JFf9B; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf09.hostedemail.com: domain of yajun.deng@linux.dev designates 91.218.175.199 as permitted sender) smtp.mailfrom=yajun.deng@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696343900; a=rsa-sha256; cv=none; b=Xes7s+zkOIXbp3DDREKpNqBnmJkUf/7JQXoMeE/+SG0QbVV0LohR5HLxdibHf3Inx4KnC+ p/EmzIzPYpho+Rr8fgGGDM+gieSvVRldcRHRZjC27d31DA9Iq5BYJQBa7uZfczYzKMZ+Xr XD3xARIlXRJrUP7fSs0VJVLy1OhbWlE= Message-ID: <8c9ee3bd-6d71-4111-8f4e-91bc52b42ed4@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1696343898; h=from:from: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=NtOgS8kdqdev2oPEOGg7PvbjyZjOnfyVKUNFOralQmQ=; b=ci+JFf9BqXsUYWMEsTqZOYEE80uqO9eDp4Uhvo68rnHJZPASLc2gn+1e953ZBMTII5+HkH 9DO1bccuQdRD07co0hGqNjqCfQJca9+yEuje63CG5tCN9L8Wn3/5V61aSbY8fb3/TGU6rh urWiPBsB8/7TKLCXcM6CPLqigyQK7WU= Date: Tue, 3 Oct 2023 22:38:09 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v4 2/2] mm: Init page count in reserve_bootmem_region when MEMINIT_EARLY Content-Language: en-US To: David Hildenbrand , Mike Rapoport Cc: akpm@linux-foundation.org, mike.kravetz@oracle.com, muchun.song@linux.dev, willy@infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20230928083302.386202-1-yajun.deng@linux.dev> <20230928083302.386202-3-yajun.deng@linux.dev> <20230929083018.GU3303@kernel.org> <20230929100252.GW3303@kernel.org> <15233624-f32e-172e-b2f6-7ca7bffbc96d@linux.dev> <20231001185934.GX3303@kernel.org> <90342474-432a-9fe3-2f11-915a04f0053f@linux.dev> <20231002084708.GZ3303@kernel.org> <20231002111051.GA3303@kernel.org> <3057dab3-19f2-99ca-f125-e91a094975ed@redhat.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yajun Deng In-Reply-To: <3057dab3-19f2-99ca-f125-e91a094975ed@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 910E1140030 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: rmidibek1fqaqid18kyyfkz8fb5pnrwe X-HE-Tag: 1696343900-877924 X-HE-Meta: U2FsdGVkX1+LSvzFg8/omy5zbLR7xFpX6+L9a1CVrTldyMYrrFZBREXCrKYShaalIUqBVhIrlFdH5oXxTZ4FUVO7sgp62meDBTkqgO469+IYb1Bj06BLGd61aNgXEtEnGQ2jaof1ZSsBLNghjG3lVXSa4+P9LvnkoUHSM+chiWrzBHf0goAsRtP9WKlgVkZaJpRZIpOV5KZa6gmlTV78tSonw6ZxQ1+hVGmpS6V8RkKtcJm/POAuaSHDu4xXOPqDeMO/ZYJPcoa2ySsN4ka0ERGox0cmyKgLjNQPtbPajZnagdUaIkl9nVy3sHAEYMMrPvDYlDWsx+tJVz4y8HSzG4pMS/OMdOrk8uITtvut2Hm8QtNhVNiSx28UGKUbKzUlja4DjhRC9I/sXTUkvdddYJNHbJtM/a4or+jJbBz0idmIChp8RqIOJxW0bZbDlR3/4j48xgP5Gi9o8pjgyWxhXuaZqJ3MsnWqJo2pDEK48TWs7IN8bvz/tNey8CuZNL6IB4owH8nX6uz6xbT51439CjVg2F9Ryi7YAoMxkQbU6o5iB2zcR6aDfqwvLEgO9ZaPYuqahbwn6NMErxc0aYrMwZGzrwTlArQpiDLqSf1HXD6Ze8dbUqDTU5l5Tdc6fgwZ7AZELVF9TV7e9Yz715csYA+lryGDriv3Fzmd47T8V0YHjdwkhpaulg9STc61x88BBgdTtTPCXStWCKQP4UZHndLPV15VkXNbRXbtrq4i2ZPJ3A/6ft09Y2ncaTcoMX0lyzDWNGUwVC6kHg/5c9sJ+0shX9hQc1EtcHs/Hwsmy02PF/deKr0+PhTwHw+1tRwEb4A3p4FY5QClgq+k8L4fgr9m7i3DPaNNZchCYAtodhM+Bd5FJIw/wyLg9ie4cA/w27sXimDQ95oD54DWUz/BEd9v1reC+u1dM+skL7SJZK6qcZSeWHVhDLc92UxeIL59oVD1VVoukmN5VGPEAhx pGDi0n93 +KH/BqhS/Z0N9GKv9Qep4oxy22nk6u2nhu7x5cPwfbEZAJ9i01tFLn+E23VhEVWRLPRzIA5tr2xZxMqbCJ56uEldjQN+0SSB8AI6bGqSznixrZQbNQVqw025yabkH8Dg6IQV7J+zkt7PIA8U= 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 2023/10/2 19:25, David Hildenbrand wrote: > On 02.10.23 13:10, Mike Rapoport wrote: >> On Mon, Oct 02, 2023 at 10:56:51AM +0200, David Hildenbrand wrote: >>> On 02.10.23 10:47, Mike Rapoport wrote: >>>> On Mon, Oct 02, 2023 at 03:03:56PM +0800, Yajun Deng wrote: >>>>> >>>>> On 2023/10/2 02:59, Mike Rapoport wrote: >>>>>> On Fri, Sep 29, 2023 at 06:27:25PM +0800, Yajun Deng wrote: >>>>>>> On 2023/9/29 18:02, Mike Rapoport wrote: >>>>>>>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>>>>>>> index 06be8821d833..b868caabe8dc 100644 >>>>>>>>>>> --- a/mm/page_alloc.c >>>>>>>>>>> +++ b/mm/page_alloc.c >>>>>>>>>>> @@ -1285,18 +1285,22 @@ void __free_pages_core(struct page >>>>>>>>>>> *page, unsigned int order) >>>>>>>>>>>           unsigned int loop; >>>>>>>>>>>           /* >>>>>>>>>>> -     * When initializing the memmap, __init_single_page() >>>>>>>>>>> sets the refcount >>>>>>>>>>> -     * of all pages to 1 ("allocated"/"not free"). We have >>>>>>>>>>> to set the >>>>>>>>>>> -     * refcount of all involved pages to 0. >>>>>>>>>>> +     * When initializing the memmap, memmap_init_range sets >>>>>>>>>>> the refcount >>>>>>>>>>> +     * of all pages to 1 ("reserved" and "free") in hotplug >>>>>>>>>>> context. We >>>>>>>>>>> +     * have to set the refcount of all involved pages to 0. >>>>>>>>>>> Otherwise, >>>>>>>>>>> +     * we don't do it, as reserve_bootmem_region only set >>>>>>>>>>> the refcount on >>>>>>>>>>> +     * reserve region ("reserved") in early context. >>>>>>>>>>>            */ >>>>>>>>>> Again, why hotplug and early init should be different? >>>>>>>>> I will add a comment that describes it will save boot time. >>>>>>>> But why do we need initialize struct pages differently at boot >>>>>>>> time vs >>>>>>>> memory hotplug? >>>>>>>> Is there a reason memory hotplug cannot have page count set to >>>>>>>> 0 just like >>>>>>>> for pages reserved at boot time? >>>>>>> This patch just save boot time in MEMINIT_EARLY. If someone >>>>>>> finds out that >>>>>>> it can save time in >>>>>>> >>>>>>> MEMINIT_HOTPLUG, I think it can be done in another patch later. >>>>>>> I just >>>>>>> keeping it in the same. >>>>>> But it's not the same. It becomes slower after your patch and the >>>>>> code that >>>>>> frees the pages for MEMINIT_EARLY and MEMINIT_HOTPLUG becomes >>>>>> non-uniform >>>>>> for no apparent reason. >>>>> >>>>> __free_pages_core will also be called by others, such as: >>>>> deferred_free_range, do_collection and memblock_free_late. >>>>> >>>>> We couldn't remove  'if (page_count(page))' even if we set page >>>>> count to 0 >>>>> when MEMINIT_HOTPLUG. >>>> >>>> That 'if' breaks the invariant that __free_pages_core is always >>>> called for >>>> pages with initialized page count. Adding it may lead to subtle >>>> bugs and >>>> random memory corruption so we don't want to add it at the first >>>> place. >>> >>> As long as we have to special-case memory hotplug, we know that we are >>> always coming via generic_online_page() in that case. We could >>> either move >>> some logic over there, or let __free_pages_core() know what it >>> should do. >> >> Looks like the patch rather special cases MEMINIT_EARLY, although I >> didn't >> check throughfully other code paths. >> Anyway, relying on page_count() to be correct in different ways for >> different callers of __free_pages_core() does not sound right to me. > > Absolutely agreed. > I already sent v5  a few days ago. Comments, please...