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 19A2CCD1292 for ; Fri, 5 Apr 2024 01:51:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 847ED6B00BA; Thu, 4 Apr 2024 21:51:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F8AE6B00BB; Thu, 4 Apr 2024 21:51:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 698896B00BC; Thu, 4 Apr 2024 21:51:07 -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 4B0866B00BA for ; Thu, 4 Apr 2024 21:51:07 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 0D10E1205C0 for ; Fri, 5 Apr 2024 01:51:07 +0000 (UTC) X-FDA: 81973800174.13.BC522BC Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) by imf12.hostedemail.com (Postfix) with ESMTP id 12C454000F for ; Fri, 5 Apr 2024 01:51:03 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=XeqlM8MM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.218.44 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=1712281864; 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=LG9zrXnPXXTDaeJWUYkEnV/p7u33CZ2JxgE/NBbImWY=; b=3VdmwY14Uo30k63fmGtgn8sSUchWCFUVYElE3Zg9FSBpXC0jjjHZ3dYb9dUXuYxwTa9P77 YSohNzuyecKhZm3RwaMCBAkMDmohFEq6+RASBOU44f89vxAibxy+nuXyGP0gE/SCkOUPaL QKGTJDlU6ipd/mtEuEwGrj/ohN/61mg= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=XeqlM8MM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.218.44 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712281864; a=rsa-sha256; cv=none; b=s+6oJ9QEOFh5o0NBqOYXg6Vd9SdUARtuGq7bSvu8rh+GJLOWXAPjSDubJ6Zh4IEViGR8/O Y4tkvqw6A3gj9ZCsOLP2BVfV+6oIaPVKe6CbNF5Ado2JizCCW9olhlF+tIj+MntDVxXAec 9NkMv5u/0U0hr9mWFjaTR/K68UtRC3E= Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-a4702457ccbso224344166b.3 for ; Thu, 04 Apr 2024 18:51:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712281862; x=1712886662; 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=LG9zrXnPXXTDaeJWUYkEnV/p7u33CZ2JxgE/NBbImWY=; b=XeqlM8MMoZLdTyHj3MeC8WBakp637D5l81pWYjAZobPuS222/mwcCtc7kuCg34YPIV X2ngKDv/vjrtINgzHufyPZ9VpCK7/gGCzgNdwwYt4Vf55AcL8RfxaouCLXvsBqZf5JX9 y0VfuYQIumOGsM3ebfzK43PyDpENCzbeVFoD2Jp1bCg6wleDQeHmo0OjGYWDbVIBFhxu NEIW1Oy3PSF5XZMoCn4waYIVudGnpwYXkZk3gPBQon1TePq8zNuRZQBHnZFVxEEGdakd SI0csyfAKwtvo1DzdD2PpQZ/H53QNEB7c4Ti8RmksYOGjqi9s8x0IC59mWc6PD2dFWQX tuQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712281862; x=1712886662; 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=LG9zrXnPXXTDaeJWUYkEnV/p7u33CZ2JxgE/NBbImWY=; b=wnsBzwSDad8oj74/KJyHI8nZ+ToLjsAs6qe3rK0JyOMox67Qx7mqBvdJWGYC0GyUkx I52U0gUnyV+Y1/slzKSzjKEw7JDLKE9QVnBdCxKeJx9el3sZFNWyXFL4BPa9RrqxYvuM XVIpb0RYJ5hBe6gGP6cDEDZBXNUyIvN57XkA8f1QVRvWPNhLknfbuvJyEsRbkctMzfV7 wHIQy+o5B+yZiHzEnyphr/7EEc/RtfVkwknAAhjG/dE+iFCR76T9XuHiCZU+jczoLkBm BYZpbLGmN1+1bmRStSPkkViOToSfjO0NPUd5rskYxo/9MUdDsRz5zImVFm6cvtWi2Vy7 dzsg== X-Forwarded-Encrypted: i=1; AJvYcCXcxTXEmbnwbwEb2hpPSLZp5LWzb06oyveh6GSyyxhQy7mIDiNsNa5Wbtw0hVX4DTMs3rXxq39TOmdwOM9gptaVzhs= X-Gm-Message-State: AOJu0YyvdmrlaithTGmSqZTcHpRkhfBtNwXqUZ1v2BPEnQ8vArIBhqj4 yvxt7mvJpOFjmWSY7NZysYilmT3T+9NSphhtY/0J3Ekmoxkol5e2 X-Google-Smtp-Source: AGHT+IGEd60ELq9VKfKms3O12/x4Q8yi9vzwuJ3Gw+7aV7a++Wm6xsGkxcsvdVpZm8JC0X9cLARs3Q== X-Received: by 2002:a17:906:3452:b0:a51:943c:b790 with SMTP id d18-20020a170906345200b00a51943cb790mr723042ejb.66.1712281862290; Thu, 04 Apr 2024 18:51:02 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id b16-20020a170906039000b00a51a5c8ea6fsm130579eja.193.2024.04.04.18.51.01 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 04 Apr 2024 18:51:01 -0700 (PDT) Date: Fri, 5 Apr 2024 01:51:00 +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: <20240405015100.fdo25pdemy7uquuu@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: 12C454000F X-Stat-Signature: hee1s59dxr5e163in78eddcxqwkrwp9t X-Rspam-User: X-HE-Tag: 1712281863-414612 X-HE-Meta: U2FsdGVkX1+rtZARxu+d+bP8kMEQZMMTopmVVLQMRhBRdR3FknlHAPSt/GVJrtFB8t0ImlIu+VDIOMmP04ed/vh7ErcpfZtUYzyNeXg7mJuLDCngqsP2tYL7T1WTCkBPug+EID3mYXsxcKIGHoVnio2M9vrPaKV2exroXBuSwCHUqgyOs/npxq4jRXBqsIDzzMWdFoEDR81o7e1qJAnRAyXZFnLyWJLtWzORJKRBkpt2vngRDXH0Em9nO0LRdsDfADE9QrycWnWYl9zn/RPX3wK5Kh5j1n1osUKZGyS7nZ9kMvTXuk2Z9Gs5XeJ41755lLNhfAZ152jquZLGDxA+lUcvTE3IFohKai6TRg7ZlFQ33u3uyh1wfZyL8fHqrx8qZ4ISM/mXeQoO38YCeaadAk/641vsukWyKdggbDxVbhE1PhxU0zBoai2647SOYdPPy4dkvU4GgZMl9lJSVlcjpNeInyFsovbXhFiiNiWrbw9U1eTVH9z0KEtqtfCBcb1UaG01hA+xP553h82vQU3V+Teyxvt2YmjTs5WepDJv7+DUQ4VQV2/r8Znej9FRH1x1ryvCVg+JgMfmUEuWZox/WNanDYraD+iBPhmmAjUM0ZzXWy1Eueb3jjeWz7BvXDJNQGHWpb3F+IIvTGSAbVQrlDS11P5V5mqeC1p0peWHKjCxiElpeUIIFfdK2gAoLzePTl9nbfdtrXk6iUoY+39YTA2MYO4ladgUIasvOCq7wYPVfygKWybvEI8HHhOZvTjlqp1EfYYauRXENa3fc6cDcauGvlVC1/LE899Cicf9qu8FABJAmvEpm7+nm5E3NNIq8aNpu1BgtRGxg//nsOrO6MW+UYm4kwgAINUy88u0p1llUh3qUsvU4df5sse7RhSaFVaetP9dFu9ElHx+whtxoxYZTW70ZVa/ImJY3uXAwy8kgME3xNK5jJ5d+nnp5P0mj6C+1HT8MOffhHT0t1e y9RxuD// dOAk9VIuxQEDKMmUAnJ/2m1jSCz4/Sn0xFFWhzuAB0XFa+k60toUxSaKPMbf3QV1j7xcGAjX8IQD8fYZbooRpkS15x77P6hKry1tOL5J0Rc5ZoFGE+uSFMo5+X8bd8urHRBM0KDxOTmYmo0Tt1KpwekUPm1DQhjdL3UlNMnlzg/scgxSLrpfyNv0t03ZFzZ85hH5DGqoMT0JfZ070tIR0Q5V4GBuib7Maa6na40VBzdArU/BUgRtJCrAjn/+EMaUGZz0zuRLIKla8d1YrPDcvyDtv9O0dmKJzIWWaSqeFY8O+y25ywp/eavpCjnf7U6R41wj92wXQFpofBMh93FtLaVwGCM5LzuGuNZ2Rzk4gFTJb4rIyTMCpyWiaKx7zCD8xoi1JvzROs3/QxrgL2S+JJgjOyz6lA6XRwInSDb5fWxX4MmtQ7RmZS9GVbJi7OAUct8T9uYvF8KvJgZcTzIrBrHD/sa2/bRmmpYS3QEXNBpyTmOFy6qepp9isbINOMTdsOAcvEUzfw1YnnX4mItqD+ywNgyy9y6j0fEQAQikBt22XSdU9Qe6Yz4/fRX4kok+c+b2zDYlc4868RLKLjJBs45kPenvaeBKCYcMd2lcCltrOuzxg+oya5JHB0zjADqhfRIODBvbnyScvQ/C33KnDoonDgwlATszIv+BumUpuqAHSPXPmtwfni/Df0jsa1r1lT3qHAuccNW/jvVWRqwR6t2EmVXfC6mP5bnKagMIPylF+2qxKCxy2Z9wqqWJPVkHX5RHNgfPTUVFKFIOavtDV74nJFSQ9ZKKnf/qxHQyhpRNmdvJB5dRUk2V2IpaEb4jup/t6pHN0L31NJw8= 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. > Oops, I found maybe we should keep this. Since reset_memblock_regions is called on each new round test, we should clear it. >> 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