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 04C8DCD128A for ; Mon, 8 Apr 2024 05:45:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 487C16B0085; Mon, 8 Apr 2024 01:45:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 437C46B0087; Mon, 8 Apr 2024 01:45:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2FEF66B0088; Mon, 8 Apr 2024 01:45:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 125696B0085 for ; Mon, 8 Apr 2024 01:45:36 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 9C9DD1606B8 for ; Mon, 8 Apr 2024 05:45:35 +0000 (UTC) X-FDA: 81985277430.19.DC90305 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf04.hostedemail.com (Postfix) with ESMTP id 42DAC40006 for ; Mon, 8 Apr 2024 05:45:31 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=V1oBSot4; spf=pass (imf04.hostedemail.com: domain of rppt@kernel.org designates 145.40.73.55 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=1712555133; a=rsa-sha256; cv=none; b=NqDs7L5eFTR+zhYSnKhpqU8AjK+zYR+1Xy4lbqLE55owaQiHSKW/6DGPIAABEcO01TAXL+ NzTIiU3vqLiotZcmMmqYIorPFEP5FnVaHFGscDcIYUDKnxAMiVmF2ivY0CiDT0y5yQz71+ jhwAORL0q/crOV9zc0jwTM7OtAhnHzA= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=V1oBSot4; spf=pass (imf04.hostedemail.com: domain of rppt@kernel.org designates 145.40.73.55 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=1712555133; 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=2b8RTsF68AZewpMjzlJv8rEEdDIzopYZ4JVB6i0MdDc=; b=D3pav4UCSK5xeCYRvtZKnWZisTac3ASfbpcoefK/YNTPbU8/zus8OKVTzoIE4U0e8Uobwy lJKxU57QM/XdJEerZjvxHbLMyd35YrtQr0OoGqgSZu+4eursbM7Xq6KAYlGd/VJUsj9ym9 6jS+ol7GilMvu7VJsCJ03DygkgkaitQ= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 53D1CCE0B43; Mon, 8 Apr 2024 05:45:29 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7150CC433C7; Mon, 8 Apr 2024 05:45:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712555128; bh=6PZmKqlDshaIkQQnVd2GKLdGZNqHrDU8KhI5lpRfuCg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V1oBSot4JmpNdXwMrixjn8WuizqlIGmVs8MW8UQmepKYYSjSuPD93YyY3KZ00d6Ld K09MwtOPoh6s1zpumkLm/EV/Yh8eq3oEWQeqp2xRT5xHlOz0ze5iRr/nA3sowBahX/ 1uj0wRlW1Fey8IDGI+OMw1ioqOY34bDkTNSWghUHbzpjch/ilWmfS557YgBoVU2WEE /vPOxO/aVrQSWoF6uQO8KkXd/owRwB88tMib9x069sUVzGUUuCIqQaym4sbYI1oaCK OUS+K/Gao76nGiLRvYy7U6ToXtR0kul0Ck2Zou+vp3gps1lUw8VUhUV0MoZkZcFRHG YzN+SwwJjbp+g== Date: Mon, 8 Apr 2024 08:44:31 +0300 From: Mike Rapoport To: Wei Yang Cc: akpm@linux-foundation.org, linux-mm@kvack.org, Paul Mackerras , Tejun Heo Subject: Re: [Patch v3] mm/memblock: remove empty dummy entry Message-ID: References: <20240405015821.13411-1-richard.weiyang@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240405015821.13411-1-richard.weiyang@gmail.com> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 42DAC40006 X-Stat-Signature: jknba7m5b1xufjmiwfmwmjz7q787tzef X-Rspam-User: X-HE-Tag: 1712555131-47839 X-HE-Meta: U2FsdGVkX18768kT97D9F3jNWC97YBrP/ZAK93JxoKMbwbaUuzThAuG3eNyMtgUoLbSJKKZNOZYghEsWJgOPSuPG+hkud0Sw/yx5wWkN81xYzD/+/x9ThrP5Xd8yS2JaJ9XmsNPvyuZLfqbbzMNrj8RBaDoKkeH3Y+5//usw8k+VtYuNjGq/XPkF/hEeIf1s0AQvWUVROiogwUWhIaSGzXJnC6dA+6C9+1cc8Zsfl9c91z7BE9hlyguI0tCIqwqKsIOxBUtBaKx8FveTFOIrKJYXhdySXKAbMc2TWlSmqpdW1ZclfbGprMVylZZwWlmjeqhi5DInYbZ/Nv1xN9+ftalVikmfDkR0mkUk/LZT6kzIhM6R4+44v1lFKNNGfEHIhESkeDWAx7PM04Qje6jZU5Pro7dgfsZxjjpIWd1+Pc7XOUk0oMM/HQOcR86Q66IuPA/HOXxPLzS2dvkibKOt60pens5pApEjYeuzMLyg7wP+1cilq2aEsYemxitgimtzGkLGJD/RUCEqhIRS1ZcDZSPb5Ca6GWnbOPEvKemKJEbL94FNiiLxIjNJVFekmGZLaZZGQO15nV9wp1xNDB1Istvp1vvaLWWdefq1rg+MMbmKqB2/7G+Vn6UlEJ6xbXvLFwPr5dKeAkLQHpQ94/7zekivTSc6eyVB3LW3q44cnF5wosNFRKH3ccqcqnp5ZY+GNmFW82Q12Kcn6KEV4fHfocFmGGJk8F9N/NALqvMb0KZAMKhh+Sysx4WZIWbsPZTVr6Gmoci4XcE6pQ4DBIzd779kvLart9zQQaX8FqJ5+CNjv0W8mN1b9mYzUG+WsVtHR0BkuowG+Kx0ICxsbQaD/yVw4P7F5ouQ+ThgKhpJDAA2BtI2FAaGiLwXIqlyQ+xdSAXwns65UXveHKP4rNqYOEXBlyGFUfM+Q3iq15DU+tdHTkNZLLs4xviz8suorl4X/m1yqLYVqisMSN2ARsE +qjvb9Oq 2J54LUEikeE563/aoWBM/QXokUC9xnZalhW0WuaHhy3c2fawSSBEg7he/eCHjd5J2S0HvHoXlMr6UQfP6ko3UShNqrgNfyLwgcB0BYkIMdOX9dBAoDzkrcWxidjE6VqO7q8Mf3FkSdGlqeXrBombf44T8f42e1QtusxiDghEopkSd/KaEY29s8LaojaFIk/uiokrrO1MLp92QCWGdKeCFwO4dw5eMIGtT36qSeQbRhXnyJpGPXOEwkFS+yI+n6rO8Zjygk7oqRwIaAtgi3Ys1BNr5fQHOoNNWXVe7Gc5aj08+lIgl/gl2+p4fGBr3LMyKhXOkR9NNi3UtXc7lU0os8JQrX45qPT7iMekipXVf3MwW6ZPZd0VkI1Gf5xdfAyPat7pWEbgPuB4WEVN+uZeymP75sg== 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 Fri, Apr 05, 2024 at 01:58:21AM +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 > > --- > v2: remove cnt initialization to 0 and keep special case for empty array > v3: reset cnt to 0 in memblock test > --- > mm/memblock.c | 7 ++----- > tools/testing/memblock/tests/basic_api.c | 8 ++++---- > tools/testing/memblock/tests/common.c | 4 ++-- > 3 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/mm/memblock.c b/mm/memblock.c > index d09136e040d3..98d25689cf10 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -114,12 +114,10 @@ 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.max = INIT_MEMBLOCK_MEMORY_REGIONS, > .memory.name = "memory", > > .reserved.regions = memblock_reserved_init_regions, > - .reserved.cnt = 1, /* empty dummy entry */ > .reserved.max = INIT_MEMBLOCK_RESERVED_REGIONS, > .reserved.name = "reserved", > > @@ -130,7 +128,6 @@ 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 */ > .max = INIT_PHYSMEM_REGIONS, > .name = "physmem", > }; > @@ -356,7 +353,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; Sorry if I wasn't clear, I meant to keep special case only in memblock_add_range. Here I think WARN_ON(type->cnt == 0 && type->total_size != 0); should be enough. > type->regions[0].base = 0; > type->regions[0].size = 0; > type->regions[0].flags = 0; > @@ -600,12 +596,13 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > > /* special case for empty array */ > if (type->regions[0].size == 0) { > - WARN_ON(type->cnt != 1 || type->total_size); > + WARN_ON(type->cnt != 0 || 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; > + type->cnt = 1; > return 0; > } > > 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..c2c569f12178 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; > 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.