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 4D35FCD1284 for ; Thu, 4 Apr 2024 12:44:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D4FE26B0098; Thu, 4 Apr 2024 08:44:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CFE8A6B009B; Thu, 4 Apr 2024 08:44:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BC62D6B009C; Thu, 4 Apr 2024 08:44:29 -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 9E4626B0098 for ; Thu, 4 Apr 2024 08:44:29 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 517EC140D59 for ; Thu, 4 Apr 2024 12:44:29 +0000 (UTC) X-FDA: 81971817858.29.FEC9B3C Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf03.hostedemail.com (Postfix) with ESMTP id 7C76820024 for ; Thu, 4 Apr 2024 12:44:26 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Qj/x6QhD"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf03.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712234666; h=from:from:sender:reply-to: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=SHR9F8gGm1UpMsz/fQHU3dv8dX9k1kMI0MowmL15cIE=; b=RhZIV1fcQt+0xzxpzgH4ztIFPB+iaoUKxU55Ihd0HE2lIQ7XPCFyqAHkIymOB5sePa9Ktg YIG5e2gb9Qw6rGARWAXbHOJqL3Xuh5WyijAPw98Uh2ykjnbfsrECGL7jbyi1b595DaHl2D goyAcFA2Z3xMU5DvvYCbXhIX3QWNv4s= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="Qj/x6QhD"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf03.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712234666; a=rsa-sha256; cv=none; b=XgrVdWYG5oZWpBiu/lJ2GWrt6lOfz1/cpVYm43b2DCjxeWs/2ARWUaM7VsiRtgpvShLpXX Rp/Yh0VO9tgkEwvyzeF2O6FslIsyKWHRW4/aZAxdGkhcyIJrG5J4ivvbY5M5PqVTsm/XmR muh4+sttzLvrIUswF7hACKvqskbASKA= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a4715991c32so144889566b.1 for ; Thu, 04 Apr 2024 05:44:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712234665; x=1712839465; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=SHR9F8gGm1UpMsz/fQHU3dv8dX9k1kMI0MowmL15cIE=; b=Qj/x6QhDYj5vhbb7Ds8MsqHCkF1s7oZO/Dv/kQJKWyY0qqyfzgqEhqR5k01P6zbcL8 q6hh856lFRM3PeBCPcMxVF9PbekK1Gk08Bjd/G+DEOAnukIfqNTjR0+DTn0xFIJ8m9rL bq68mKI6yFamN6pJYXiA0ssC/eYs4egF1AV0RzOBI7BTpEB/Bn6OCuZQx/YBaGB71DVB XASY788c7CYd8+y3ugHI0/6L2BkYWGK8rfxPMpCGGsyVTIASW2lkbZL7AbXLh3XkNOgx IN5PwEfcjnsgTZE9ppyM3fqJqX2pBJcgqcRtDJtvEuS2EYIx40lt92YJG0j5w6A/fk6Y t1jA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712234665; x=1712839465; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=SHR9F8gGm1UpMsz/fQHU3dv8dX9k1kMI0MowmL15cIE=; b=T9hr/Lz41SYLvLQxMbwoJraX3aI9JzjMuEUFAf2uMTVFmxII/9BnXoJPlA3seg/R0h vZ7ZyWB+cEjLSvSdQdUr2Bg1TDT1KCiu+h1Chf9wGurLHlr4hSFPWKxAnF9/UbUt8vJP 16qL6wkgA/UHqPuo5HLnZABXhREBaHskfx8QH1IlyXLw3BDqJQ1G5/BHegj6p62L8C2l xEx0aw5NJQh9dSxVenqzWKQ4O7PoWvkfkqwBkh+0hNK7z8dMvIV/jxn7X++7ZBwnwuyj tgjPtb0kxSsspHpXh3TJCt32Wi1MfBnBeztz1Vrn9FRSVwT9GlRGgH5jYDnnx2xQzIgP cYpA== X-Forwarded-Encrypted: i=1; AJvYcCUycYwGdYxC2PGDoEtj7QaIAnDfY1+NT0nphj2hsMXuy5QZWd79V9kGvaQJQlBqrFzvn/Si4hV5hFokPyQKAABqdT0= X-Gm-Message-State: AOJu0YwxMfeZML85Sz4mbMP8mW5aIpJ6DKqwIUg/CMRj9kFuGqyV3Rv1 jyztzZaVJwr+uUaxc8OA0cHcsu+cfule9TXvpZlMlmuIEzYL4g+U X-Google-Smtp-Source: AGHT+IGkJITdwKZ4+bNX2pAzWONuRmRyAyYNRb5uylMyhTvcjmrvD+u7dU+LWtI8FxQjlcVc7izyOQ== X-Received: by 2002:a17:906:e0da:b0:a4e:9b06:db52 with SMTP id gl26-20020a170906e0da00b00a4e9b06db52mr1504238ejb.45.1712234664654; Thu, 04 Apr 2024 05:44:24 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id bk24-20020a170906b0d800b00a46a71c5324sm9050724ejb.34.2024.04.04.05.44.23 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 04 Apr 2024 05:44:23 -0700 (PDT) Date: Thu, 4 Apr 2024 12:44:23 +0000 From: Wei Yang To: Mike Rapoport Cc: Wei Yang , akpm@linux-foundation.org, linux-mm@kvack.org, Paul Mackerras , Tejun Heo Subject: Re: [PATCH] mm/memblock: remove empty dummy entry Message-ID: <20240404124423.vjsvlfr42d34ftme@master> Reply-To: Wei Yang 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: User-Agent: NeoMutt/20170113 (1.7.2) X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 7C76820024 X-Stat-Signature: rmihdpsiorkh37djnr1nf8faidpgxaqc X-Rspam-User: X-HE-Tag: 1712234666-39904 X-HE-Meta: U2FsdGVkX1+lumlbcez03ef0N2nvomlGePRyBUoBJO1uMMrK8XNmrXrNSAGgqSvzo2lAp9TuE437t/VzhtVIMFN9fUr8+wQxLlnVhDjO1LdWliMk8UpVicl4GXAmXT035F0/2IQrue1Mg+BDOoEUF+t1uEAti5lJ6HiM+W1Ai1X7mXhikfyy89XRHDLxAOKDPS3/k5K6LvxMAmfzTHE2XjVVJ5iwYh/yy39p0zj9cUfg2a0mNdpZpyJCpPswPI7beoBYUcD6KDQZbzPoIPHIIY8S0d+qtQOuSSQ5IVbc5yhaqcCvb6bv9q64gmonrJnzo6xweKO1+JCsr416dubKwXI4DOdMNnMXHUt6ozZcWkDa9p3DDFAYhNimWeQ8MOatQZycIEGL2SAchvFCrT8UT2zwPhnAahaIowiNyN2rzN3uQW0zMr8Y64meV7Iu/pcN5IDR3uxOrrXLLq52WL2sUra9/8bFCXJmr5Bidw9j8NwSmcLor7+rTlTDEruFcdP+vIRkbbe9UfHh2gdD+wGIgMHDwzPoPR0hHXnY2JGquBMPpGS/N/eYuKmGFq1XETY1HFKdQmissNwamNr7MAb02QZRzUsA04TwN3G8iEGFdCMYq89TYGsB/07TH0BpyXO5t19f3xKaDR2M0LuoWdApJI5HebkwapHYsaGkRF3KvpE7itgMBWZLxdBLvMEcXeQiNSOMqwdXEgFPTbxFi1NNPTdUNiPbF+NoQIHG1kByBK0DRYcJH9GBN0ZdubFniHHKqLTIMyBfYYkSkMi5K3fXh+A1BFygz1mQdwlVJC6HQisLz4R0tm3Xl3mTgfhK66nwz7AA1svm5Cg8NcS9xKiiVB4u1Ny9HaUBzelXCDhqHL1yPRS53v58oxIhfMvx8eGubgxKfeiQuTVf5GDkIStSNiD3z7ocZ3rZmEf12Bkrm2zg/H9uOf/FtgQpfMdzcfb3R8ZT3wa8KcADTPa7OU7 ADHnRj3P t8eNO4qECuuMFl1gdHoPJskRS0FGDSG6yIjpwGgKeWS8wAA6riZx9APxJtyu+t5brdtL1FWFsWyCtt8hLDQzeKoyyDXbJZXi/YHme5nYfymnNENUJkrmKh6Wq7Ulyfe/VsJfck7Tyng9pCi8bb1HSKwj0bWPM1TfKvWYdGQuQ+AD4koc5mNjOOIMaTyoqYcwYf/uXk3vT2jMTcybXCqcQZCC8k55fthlTpAmNsCbd6caeoS2gQJigVrdT8XAC9oOCV7XOVbab368+yLZzJEmgW3+7tA== 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 Thu, Apr 04, 2024 at 01:13:41PM +0300, Mike Rapoport wrote: >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. > Sure, I'd prepare v2. >> 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. -- Wei Yang Help you, Help me