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 DB43ACDB482 for ; Mon, 16 Oct 2023 08:36:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 486658D0051; Mon, 16 Oct 2023 04:36:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4372F8D0001; Mon, 16 Oct 2023 04:36:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D7388D0051; Mon, 16 Oct 2023 04:36:25 -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 1D9A38D0001 for ; Mon, 16 Oct 2023 04:36:25 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id DDC2CC0B17 for ; Mon, 16 Oct 2023 08:36:24 +0000 (UTC) X-FDA: 81350667888.20.56F2D06 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf28.hostedemail.com (Postfix) with ESMTP id B6833C000F for ; Mon, 16 Oct 2023 08:36:22 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LZNv5Gta; spf=pass (imf28.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697445382; 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=9HMKK4q71/SJmVH5IV1VPg5kXvk5AIz/kddYNCFXCGg=; b=y0LNpDgt3V1vXjpEpfEXBzMtLmv/UAoRDwJTKarnFGZCYrJ450q8z+qWzwVP9I7LKm8XmZ /OGwljhIi7ukREC2NO+89ZgljplnOO5ZJ/XOGYab3NFUHKzBBJwbuTe9B9oCsSSL/IKJCE WTR3nrZKF+MVZAVwusLcviAxb6DlbBM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697445382; a=rsa-sha256; cv=none; b=OpcouvEAYXoohdhv4BWXcSjtkE8GdCGoeXaPS5C681CogZ9J3q+R7cxZ4mN8QoTV0KvcuL QfSvS1PyhV2hirAKYzQo1skEfEJt5IhegeoZ4WlMJRGDJKcYg0qf7hs3NeU8CV+ugv9+fB l+cRDWxXCm3hsgW8IkoeeLf7/IZ2rt8= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LZNv5Gta; spf=pass (imf28.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1697445382; 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=9HMKK4q71/SJmVH5IV1VPg5kXvk5AIz/kddYNCFXCGg=; b=LZNv5GtaDQU6rNfGTg2wRgQ/ymPqG035t68+rXgU8wBORyltbayuup9bKk7ZeO1x7f07B4 oryX3rMvoBZbuqKbZbvQiFjftvJYFte8ayXzqySoz6AUAYbmMkNdkrJ/zHjXOhBcNZgKd1 RZn9uy89k6eycgobyiZ26NhL4xygvh4= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-77-cf8Cey0RP-m7zuNhyOaDgw-1; Mon, 16 Oct 2023 04:36:17 -0400 X-MC-Unique: cf8Cey0RP-m7zuNhyOaDgw-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-405629826ccso31166405e9.0 for ; Mon, 16 Oct 2023 01:36:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697445376; x=1698050176; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=9HMKK4q71/SJmVH5IV1VPg5kXvk5AIz/kddYNCFXCGg=; b=T22siHWbo/HDtT3pOSWwNuL0WJgT85NX75L1IwZpo67/6gw+3r5tcZhf6/0bMZnrsM qZOluwmDhumpx9dglNNCI2dbF9kaoOGYFzg0UPR6mEtXYbprupM34CtpCtbGuS+hp2Iu zgRF6RrSIMAK/6UGQmaF1ii32APwfW/LoBqfhqdT0D/QsKoI+jlcU8yXyx02dRGKx/Yw BWrMHSZ5u+UGMkrrnndOuca2Jk7nFpjnLo3ihz8LI1y9sYXQYpABsUwW/ZRadnqpO0Vf Y0IbF9PXi+gwN6YOQEDgUYloHDWN9T9qzgZwqAjQvYAYUQIfzR8At/q/kNvJbn7JiV1i Jqog== X-Gm-Message-State: AOJu0YzTvsNHT+O0MciO+ZnM9iB8NIMwvfuC+xKolA2XNCo8Y5b8/bSm RPe1B1wPrTdDAzUWOFsYrpqM4vc9W17YFRnOJ1jd/RZfRYfPRLbWWWwbbgMnVQ+Y7rin2fUkdCA rkBYekeSUyos= X-Received: by 2002:a05:600c:b41:b0:3f7:f2d0:b904 with SMTP id k1-20020a05600c0b4100b003f7f2d0b904mr29503289wmr.8.1697445376124; Mon, 16 Oct 2023 01:36:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFCGcKG9jXgoUykxPTMVFoUwQQAXnZvcaaJ86QHHLdSZ62SKMRNNP5iqqvRkjUHpPXxFMrETA== X-Received: by 2002:a05:600c:b41:b0:3f7:f2d0:b904 with SMTP id k1-20020a05600c0b4100b003f7f2d0b904mr29503274wmr.8.1697445375714; Mon, 16 Oct 2023 01:36:15 -0700 (PDT) Received: from ?IPV6:2003:cb:c73c:9300:8903:bf2e:db72:6527? (p200300cbc73c93008903bf2edb726527.dip0.t-ipconnect.de. [2003:cb:c73c:9300:8903:bf2e:db72:6527]) by smtp.gmail.com with ESMTPSA id t15-20020a05600c198f00b004077227bcdesm6564802wmq.26.2023.10.16.01.36.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Oct 2023 01:36:15 -0700 (PDT) Message-ID: <23302f67-eb69-265a-ab2d-9c55715e2843@redhat.com> Date: Mon, 16 Oct 2023 10:36:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v4 2/2] mm: Init page count in reserve_bootmem_region when MEMINIT_EARLY To: Yajun Deng , 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> <2f8c4741-5c7f-272d-9cef-9fda9fbc7ca6@linux.dev> <5382bf2d-5aa0-1498-8169-3248be4b5af3@linux.dev> <38cd0cb9-efe9-b98a-2768-ccb48da8b812@linux.dev> <20231013084827.GT3303@kernel.org> <1c91dd62-886d-bb05-8aee-22191a8a2d8e@linux.dev> <20231016063357.GU3303@kernel.org> <0d890048-be58-5050-02fa-21768059aa0d@linux.dev> From: David Hildenbrand Organization: Red Hat In-Reply-To: <0d890048-be58-5050-02fa-21768059aa0d@linux.dev> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: 5xhkji9c5nn5em8qwyifj6tn17z433co X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: B6833C000F X-Rspam-User: X-HE-Tag: 1697445382-462415 X-HE-Meta: U2FsdGVkX18zfLdCLpbd1PGkDsMR2do9dfcKVwdCRgEyuB3pn+t8MnEla4fi0zDgoZP3xnCdOhaGkrhT/Ih4a7b24B+t0U2XWuAAW41KmOPtmEWm6iDxSGvXC4GgxqQLzVNg25/tqf6Unq2Du7Afif8Rc3tK4A7mKQ9ZYpKpnCWb1v+RBfa9NGAGNNMJoXcEinpRk1PZuKzkknHd4qdgzCblttamJq7ksQOdiazR59JdJyGCCXFt89w9HSBIi4dV/DaaMBx7rXxJ8vxUqwb3DFth+GHBVEWacbSLsuuK6pL+Foh/4/kUZx6BM92kKmOcN6aAGCsz25CIZOYCzmos0b3DJGeJdXfJu9recCdrseKneXZpmgiHY5zaqV7nQtI94SfHeOIh6W8bfgeMVOfOuZ7/RhoyiSS13pNrn+Y+xffFFwROVgTlujs0yq4WTaOpQHy7AdSQ3m+8bxnc5l33VbOpX+EULnKaDHKAskND3h9PQybwkAtJ5Kx2XtF6H7odyDWeV3Y2oZ4nANK1md4R7fq8M6xKACx+DBDJuBGQP4BChjYgK9+9LImnPKfUL4CIAq2FX88uklAz35K4fyNQ4S+VErmAd+VVnwusbe9+KLG+KpAvMORsTvgXp4H6ENBqtwIaiTsevrjJYT2U6xBRrRbhmwkVrFEWDcgK1MLciNuxxm/KS9P2gWPtsjwlQzK6lOfEYCQY0XLL/thgv7oPIUqkd9vkPBzut+TJvZopVeJDvDTNpgfPH0fs53h9CMQn3sJ5tbf4bmM9aY85QjCMW4ovzIdJeXk1StEVRY657Sdk8bEsRAWsyeuu/CJMoy9anre9UHLsWmVH3psohp3Fk5zuQjVJTbOIkE4Kk9eihBrB0O5usd7zxSPiRur/f2low1iBV0BYirTHeAMvq86HJ++TWk8CcmM6By7gCxLXSy+jaae9tkiuIVVJDXZl2XDtSpah/xn8kdVRfEp6WPH LlWic2Mo cSc9sDjzZF/mwRDkpT0+cHM/WJ8m1HeX9773gb/U20UCB1BuHwqb21U6g+CNbTZ1JA16MgbwvV7Bf4YSe+8xh4QiOj2g5hhKCMnkyQggC/Q282hqd3nLIH6v/Nz4YCHt0PrcPrZGgM0OmQ5Gjq+sKnQbg84TCRG4+n5zEaBWRryexJzcV8ZkLyEEkXLPTYJUi0Kl6dXyxgBh5BUyDc3HE+STfO//KXP8MfcABb08G6M+I2/0ZFTVWB5ukOij2bT7mhuS3lr6ALpN/a549pijLCRQxq8OEfCnf6vne96pLzBlr4flLsze5BDw5sdZnRRtN1iT7UbAn6QOqFYtHK/gRCpokeEjBWeCaWWn02TebZ83c+v+FZrculu4UpaNRdDXs2wT44BCkkrvmr/PjpGzXJm8p3WQASgOOpFnaRayBhniy8bNFabz5FCTX7zrK/2FW6Yd0K7dYDq0F4YlV6bEPMNXz+m275kgPPVOxlyW65pQ83vY= 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 16.10.23 10:32, Yajun Deng wrote: > > On 2023/10/16 16:16, David Hildenbrand wrote: >> On 16.10.23 10:10, Yajun Deng wrote: >>> >>> On 2023/10/16 14:33, Mike Rapoport wrote: >>>> On Fri, Oct 13, 2023 at 05:29:19PM +0800, Yajun Deng wrote: >>>>> On 2023/10/13 16:48, Mike Rapoport wrote: >>>>>> On Thu, Oct 12, 2023 at 05:53:22PM +0800, Yajun Deng wrote: >>>>>>> On 2023/10/12 17:23, David Hildenbrand wrote: >>>>>>>> On 10.10.23 04:31, Yajun Deng wrote: >>>>>>>>> On 2023/10/8 16:57, Yajun Deng wrote: >>>>>>>>>>> That looks wrong. if the page count would by pure luck be 0 >>>>>>>>>>> already for hotplugged memory, you wouldn't clear the reserved >>>>>>>>>>> flag. >>>>>>>>>>> >>>>>>>>>>> These changes make me a bit nervous. >>>>>>>>>> Is 'if (page_count(page) || PageReserved(page))' be safer? Or >>>>>>>>>> do I >>>>>>>>>> need to do something else? >>>>>>>>>> >>>>>>>>> How about the following if statement? But it needs to add more >>>>>>>>> patch >>>>>>>>> like v1 ([PATCH 2/4] mm: Introduce MEMINIT_LATE context). >>>>>>>>> >>>>>>>>> It'll be safer, but more complex. Please comment... >>>>>>>>> >>>>>>>>>        if (context != MEMINIT_EARLY || (page_count(page) || >>>>>>>>> PageReserved(page)) { >>>>>>>>> >>>>>>>> Ideally we could make initialization only depend on the context, >>>>>>>> and not >>>>>>>> check for count or the reserved flag. >>>>>>>> >>>>>>> This link is v1, >>>>>>> https://lore.kernel.org/all/20230922070923.355656-1-yajun.deng@linux.dev/ >>>>>>> >>>>>>> >>>>>>> If we could make initialization only depend on the context, I'll >>>>>>> modify it >>>>>>> based on v1. >>>>>> Although ~20% improvement looks impressive, this is only >>>>>> optimization of a >>>>>> fraction of the boot time, and realistically, how much 56 msec >>>>>> saves from >>>>>> the total boot time when you boot a machine with 190G of RAM? >>>>> There are a lot of factors that can affect the total boot time. 56 >>>>> msec >>>>> saves may be insignificant. >>>>> >>>>> But if we look at the boot log, we'll see there's a significant >>>>> time jump. >>>>> >>>>> before: >>>>> >>>>> [    0.250334] ACPI: PM-Timer IO Port: 0x508 >>>>> [    0.618994] Memory: 173413056K/199884452K available (18440K >>>>> kernel code, >>>>> >>>>> after: >>>>> >>>>> [    0.260229] software IO TLB: area num 32. >>>>> [    0.563497] Memory: 173413056K/199884452K available (18440K >>>>> kernel code, >>>>> Memory: >>>>> Memory initialization is time consuming in the boot log. >>>> You just confirmed that 56 msec is insignificant and then you send >>>> again >>>> the improvement of ~60 msec in memory initialization. >>>> >>>> What does this improvement gain in percentage of total boot time? >>> >>> >>> before: >>> >>> [   10.692708] Run /init as init process >>> >>> >>> after: >>> >>> [   10.666290] Run /init as init process >>> >>> >>> About 0.25%. The total boot time is variable, depending on how many >>> drivers need to be initialized. >>> >>> >>>>>> I still think the improvement does not justify the churn, added >>>>>> complexity >>>>>> and special casing of different code paths of initialization of >>>>>> struct pages. >>>>> >>>>> Because there is a loop, if the order is MAX_ORDER, the loop will >>>>> run 1024 >>>>> times. The following 'if' would be safer: >>>>> >>>>> 'if (context != MEMINIT_EARLY || (page_count(page) || >> >>>>> PageReserved(page)) >>>>> {' >>>> No, it will not. >>>> >>>> As the matter of fact any condition here won't be 'safer' because it >>>> makes >>>> the code more complex and less maintainable. >>>> Any future change in __free_pages_core() or one of it's callers will >>>> have >>>> to reason what will happen with that condition after the change. >>> >>> >>> To avoid introducing MEMINIT_LATE context and make code simpler. This >>> might be a better option. >>> >>> if (page_count(page) || PageReserved(page)) >> >> I'll have to side with Mike here; this change might not be worth it. >> > > Okay, I got it. Thanks! IMHO instead of adding more checks to that code we should try to unify that handling such that we can just remove it. As expressed, at least from the memory hotplug perspective there are still reasons why we need that; I can provide some guidance on how to eventually achieve that, but it might end up in a bit of work ... Anyhow, thanks for bringing up that topic; it reminded me that I still have pending cleanups to not rely on PageReserved on the memory hotplug path. -- Cheers, David / dhildenb