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 259E8CD1284 for ; Thu, 4 Apr 2024 10:14:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 899626B0087; Thu, 4 Apr 2024 06:14:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 849186B0088; Thu, 4 Apr 2024 06:14:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6EAFB6B0089; Thu, 4 Apr 2024 06:14:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 509776B0087 for ; Thu, 4 Apr 2024 06:14:37 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 057CC140F6D for ; Thu, 4 Apr 2024 10:14:37 +0000 (UTC) X-FDA: 81971440194.20.C8A6173 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf23.hostedemail.com (Postfix) with ESMTP id 5614E14001A for ; Thu, 4 Apr 2024 10:14:35 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=g25hMtXK; spf=pass (imf23.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712225675; a=rsa-sha256; cv=none; b=P8ZmbVoiJHdKH6OtJ/bQP4kjwad7uDlA1Tre4TWZU5L8rmnYOLkPyaOyOofZwZFLsmQkgX dGJACYRs0Rkee9K7HJUJIsSLz9BNriUr4yLGMvl0AYLu+HOAP6NdZH270Qc8TSRzZScnIH O/DZv2xlQqxUbijLDS+P+ngm/Lf5c9c= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=g25hMtXK; spf=pass (imf23.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712225675; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=x1zQQUPkaNxDFa7R3k0mCVkmT7oV++Oz7rAX3fi8b7w=; b=wqaeFIGl1DdVfe4YXa2NhLjbsTOH5v7R6GDySYe/ar3sKwCTlKWlbeyQ22A9EeZ9DHW5I7 uLYQaL+cRb01LhyBcV6Ignz2xd6i+UPkyflenvXfSgcl5HWR7B0TN/Y73Rx+h5mf5AkIzc HOFE2B7Nd+dqVyPF5L35D/eOxEMVJ00= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id ED5F561691; Thu, 4 Apr 2024 10:14:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BAEAAC433C7; Thu, 4 Apr 2024 10:14:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712225673; bh=8V8gZyUrVr6QarIPAojq4Bb367dg24t3Sgkp6WIRwoY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=g25hMtXK7OonsAJqYf+Ok0AVT9F+xdDRgbiyAmz99d4b5UB5hOP8DKHTz7tVQSAY9 IlxvHGnlQijMdYbx4bxsLvC+JQmpZc46RgEH6ng/oZqLg13xxhwaW1wa4N8yKkQ8hW BVsKv1s0IapwdcMtVhM391XXa84vop5l+E5Z+SQeOpWaB2Xy0dH3cJfjvGAQXnnyzf 88aOoOcoSfgxu+Vls5/cBjOcZI1S3Cze1Y10ayfTKdd9X+BcveYcCgfXTwKy0UD9RX Ddhf+0JN4a0l8PnXoZAJHIUJIP/a56ZON3qT9tQXvl3f9FV3wEC7q8InVK++iixi5q e3ERhcQAvaHhw== Date: Thu, 4 Apr 2024 13:13:41 +0300 From: Mike Rapoport To: Wei Yang Cc: akpm@linux-foundation.org, linux-mm@kvack.org, Paul Mackerras , Tejun Heo Subject: Re: [PATCH] mm/memblock: remove empty dummy entry Message-ID: References: <20240403091045.12009-1-richard.weiyang@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240403091045.12009-1-richard.weiyang@gmail.com> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 5614E14001A X-Stat-Signature: mqrngeatud4o9bmp9knfaortt8mgjqo4 X-Rspam-User: X-HE-Tag: 1712225675-448469 X-HE-Meta: U2FsdGVkX18C+/0JFsvstEEe/cdIBvU/90EQEwkAoY+nvQYV5HY78tv7dLz+qfKVYflDV42e9VM1e0Wr3rdwfEO43BmuQY8v7oBymzdfwLtcNLoATlxUXTFcWP5KBhfrNFDHBYAPYNXqnHsatvPx+XHXXpBj1go7FJ3mC25brmWHted3q8jHqlF5VGGH2FwgHEB91xIjkpGHkwsIV1YDLFR7p8BhyJJCYqsJ522w0F4G74ABvMQmbT9GivqC/te5X1uxQTsJPUsnEBnEnJDfHM0a6BJqR4vpKwO5cofYv2B2VMtWTfE7ZRqPehykHSNJpmsOn/FFsJtevMAYQDxZNtzGUZUzGgTkBG+gMp9WbAFYdTia8ZwRHgOAAH3q1KV9MngWGqfOYZYd2NQzZUyPZ8/9HOZnGE491vCioPi7RfhxjA35oXIpF1qUiS/ZQ7o+RYr5SzihvwqWPxcgZzlx2shnSmEA01TCmmZ2EvB/XP7ycWp9gN6H3tI0Q2yOkSTOEPN7JFkebXvaH5qJM0vRZQNtjcfzUy5hNv9TAJsdwU+0/oNnIogdLzYt12BBL/c0g/B0u+e0I1YWCICeKWgeElklbIL2LemaS81nPJQiECWV4Q0iml/ZMxfhRDmIYsmM/oo/Ztt1LKmW324q1gSjrwJDrTJRNzhHLMo4B7SufE1PBeJY4UQF+QNMj7qXi5q3sCJmTH2XrfYlAAWmErwmIZjmcZfM/4uwJc87H8uiezYzgvvtJ21ppIoJsXIOIQT43SbdhDK/Z9m56Pw2odOaiNqQtBvoCMNxd7u8wPjUnQhsLhOb7MtqQB07H+LNXN53TP7v2gbX3ROToqo0DeEgdjaymhvNqBdRU2LdRzrviaYnWEXalwxNprZfpcSGXOcVUEabpCU5CFtNAJ+7SMWnh0ilXOpyOi1IA2oXGI45mfIjqVycdG9dCq2DGeDUcOQTZyEBtqXFBppfME3CNER 4LBAE/Nt qt/dOtXAgb18Ua5/ONECOPfGLZWXBdHwiksSc2tc5XHoMg0peQ+THDMHwhR+wB7wNlIRoS1SaU6yJaj8buazLCzMr7TKTX1xjwexANb55ikdO/EIA0dp4ZgxC6MNA+QJR96JzU5ch42ZqE3trC1HzQpwLJSfzZLOXVv6n6sEz2haSq4B6x3hUMP9QkrbuDoAdg1aUpBGoMxiG01LzKHPPhJ0UcVvU7NjAZ5Qsuut+2DgSr/y06WvmS8efP688KE/URBqfyP07Apy8f6VWQhJ1nrX5EXTuImpbpjobQ7ZvxIudBq1sEZ0Uvhg3n4+LmdzTbBcIoV0ZOhnh5p4CJxWtHPBqztP/teSIFUpkP1IaeBgEOSTHQGr+sTERLFHUxbxCClgXDc9jasdINxS7FRPJyVzkWQ== 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: List-Subscribe: List-Unsubscribe: On Wed, Apr 03, 2024 at 09:10:45AM +0000, Wei Yang wrote: > The dummy entry is introduced in the initial implementation of lmb in > commit 7c8c6b9776fb ("powerpc: Merge lmb.c and make MM initialization > use it."). > > As the comment says the empty dummy entry is to simplify the code. > > /* Create a dummy zero size LMB which will get coalesced away later. > * This simplifies the lmb_add() code below... > */ > > While current code is reimplemented by Tejun in commit 784656f9c680 > ("memblock: Reimplement memblock_add_region()"). This empty dummy entry > seems not benefit the code any more. > > Let's remove it. > > Signed-off-by: Wei Yang > CC: Paul Mackerras > CC: Tejun Heo > CC: Mike Rapoport > --- > mm/memblock.c | 18 +++--------------- > tools/testing/memblock/tests/basic_api.c | 8 ++++---- > tools/testing/memblock/tests/common.c | 4 ++-- > 3 files changed, 9 insertions(+), 21 deletions(-) > > diff --git a/mm/memblock.c b/mm/memblock.c > index d09136e040d3..100d3b7ab499 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -114,12 +114,12 @@ static struct memblock_region memblock_physmem_init_regions[INIT_PHYSMEM_REGIONS > > struct memblock memblock __initdata_memblock = { > .memory.regions = memblock_memory_init_regions, > - .memory.cnt = 1, /* empty dummy entry */ > + .memory.cnt = 0, No need to set memory.cnt to 0, this line can be just removed > .memory.max = INIT_MEMBLOCK_MEMORY_REGIONS, > .memory.name = "memory", > > .reserved.regions = memblock_reserved_init_regions, > - .reserved.cnt = 1, /* empty dummy entry */ > + .reserved.cnt = 0, Ditto. > .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS, > .reserved.name = "reserved", > > @@ -130,7 +130,7 @@ struct memblock memblock __initdata_memblock = { > #ifdef CONFIG_HAVE_MEMBLOCK_PHYS_MAP > struct memblock_type physmem = { > .regions = memblock_physmem_init_regions, > - .cnt = 1, /* empty dummy entry */ > + .cnt = 0, Ditto. > .max = INIT_PHYSMEM_REGIONS, > .name = "physmem", > }; > @@ -356,7 +356,6 @@ static void __init_memblock memblock_remove_region(struct memblock_type *type, u > /* Special case for empty arrays */ > if (type->cnt == 0) { > WARN_ON(type->total_size != 0); > - type->cnt = 1; > type->regions[0].base = 0; > type->regions[0].size = 0; > type->regions[0].flags = 0; > @@ -598,17 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > if (!size) > return 0; > > - /* special case for empty array */ > - if (type->regions[0].size == 0) { > - WARN_ON(type->cnt != 1 || type->total_size); > - type->regions[0].base = base; > - type->regions[0].size = size; > - type->regions[0].flags = flags; > - memblock_set_region_node(&type->regions[0], nid); > - type->total_size = size; > - return 0; > - } I'd keep the special case for empty array, just update it for new semantics of type->cnt. > - > /* > * The worst case is when new range overlaps all existing regions, > * then we'll need type->cnt + 1 empty regions in @type. So if > diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c > index 57bf2688edfd..f317fe691fc4 100644 > --- a/tools/testing/memblock/tests/basic_api.c > +++ b/tools/testing/memblock/tests/basic_api.c > @@ -15,12 +15,12 @@ static int memblock_initialization_check(void) > PREFIX_PUSH(); > > ASSERT_NE(memblock.memory.regions, NULL); > - ASSERT_EQ(memblock.memory.cnt, 1); > + ASSERT_EQ(memblock.memory.cnt, 0); > ASSERT_EQ(memblock.memory.max, EXPECTED_MEMBLOCK_REGIONS); > ASSERT_EQ(strcmp(memblock.memory.name, "memory"), 0); > > ASSERT_NE(memblock.reserved.regions, NULL); > - ASSERT_EQ(memblock.reserved.cnt, 1); > + ASSERT_EQ(memblock.reserved.cnt, 0); > ASSERT_EQ(memblock.memory.max, EXPECTED_MEMBLOCK_REGIONS); > ASSERT_EQ(strcmp(memblock.reserved.name, "reserved"), 0); > > @@ -1295,7 +1295,7 @@ static int memblock_remove_only_region_check(void) > ASSERT_EQ(rgn->base, 0); > ASSERT_EQ(rgn->size, 0); > > - ASSERT_EQ(memblock.memory.cnt, 1); > + ASSERT_EQ(memblock.memory.cnt, 0); > ASSERT_EQ(memblock.memory.total_size, 0); > > test_pass_pop(); > @@ -1723,7 +1723,7 @@ static int memblock_free_only_region_check(void) > ASSERT_EQ(rgn->base, 0); > ASSERT_EQ(rgn->size, 0); > > - ASSERT_EQ(memblock.reserved.cnt, 1); > + ASSERT_EQ(memblock.reserved.cnt, 0); > ASSERT_EQ(memblock.reserved.total_size, 0); > > test_pass_pop(); > diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c > index f43b6f414983..4fa94b281280 100644 > --- a/tools/testing/memblock/tests/common.c > +++ b/tools/testing/memblock/tests/common.c > @@ -40,13 +40,13 @@ void reset_memblock_regions(void) > { > memset(memblock.memory.regions, 0, > memblock.memory.cnt * sizeof(struct memblock_region)); > - memblock.memory.cnt = 1; > + memblock.memory.cnt = 0; Here as well, no need to initialize to 0. > memblock.memory.max = INIT_MEMBLOCK_REGIONS; > memblock.memory.total_size = 0; > > memset(memblock.reserved.regions, 0, > memblock.reserved.cnt * sizeof(struct memblock_region)); > - memblock.reserved.cnt = 1; > + memblock.reserved.cnt = 0; > memblock.reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS; > memblock.reserved.total_size = 0; > } > -- > 2.34.1 > -- Sincerely yours, Mike.