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 4311FC4345F for ; Sun, 28 Apr 2024 06:42:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BB6856B0088; Sun, 28 Apr 2024 02:42:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B66CC6B0089; Sun, 28 Apr 2024 02:42:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2E6E6B008A; Sun, 28 Apr 2024 02:42:01 -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 85B8F6B0088 for ; Sun, 28 Apr 2024 02:42:01 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 2BE90A161C for ; Sun, 28 Apr 2024 06:42:01 +0000 (UTC) X-FDA: 82057995642.04.99F63BB Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf16.hostedemail.com (Postfix) with ESMTP id D8A92180006 for ; Sun, 28 Apr 2024 06:41:58 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=f2lRExPu; spf=pass (imf16.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=1714286519; 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=nZkVTqxUpZ3rycH9RtK5OtMr1qZiTCAKcXM4WYGaZhg=; b=OyFZ6+LHM0R/gpwAUyyB9T4HvjKyrs55CXjeuUmhjmAxNQDN+hz3WEZ9DcJDynebUgI/r4 gEgdTOyB/1yhqPgus5h0yPLFKlE/5e6srXXZLwRRRqDDsPG/TwxkXLODAlnEHbtRCDGGHl ZUYSO3egMw+2X3VWtZntXEhqMGmKGXo= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=f2lRExPu; spf=pass (imf16.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=1714286519; a=rsa-sha256; cv=none; b=mz/xHZBXbI+hrU4UFl2J2ABHy7gDsgvllSajB41zTSJt/exGo4+Td2kyBIku02TWTk4/ts OwcvqfO1cLsJtSnbhGY7TSYrM+BmF/fbtKlIcd8pHByiInLP4isW84iSpCbCdhEEPC9G6p I0n9OI5NrTX7KZ5GY1d2ZjE2hcSOBmo= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 29A30CE0185; Sun, 28 Apr 2024 06:41:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C1B24C113CC; Sun, 28 Apr 2024 06:41:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1714286514; bh=UMO+ttAkhVyfHvjSSIm5l1eQ+ctfak5Tw2E8tvnUJMo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=f2lRExPu0xy2867njaHF48ys35KIMlgWHOvknIPYSAMGtQYXQYwXdy8YCP9lZXgbU ScREg+pgo+etAkhYR8B+9WMVfCSabzugWW+u5+cVOmtCbKkM/tVuG4iGmyV9L7rVCi 3FLrxsHDkBORIpD2ZXdaBrOZHEdIM5tawmltpNnKg47wnX0rBfcW/ATDda/N0lq+V3 DIttwZzdUdYZJ1FqVOYxvANYdt2J9Xujh2Yck/O7ttyZyc+rHjczfiIxXAoF9/qqVb Ew/UHJ7HHBWWovQY0twmAL7VyTB080c2g9IMguK0mA3C5oIkk4p37LfWn2PAxbR4xF WK0UMDUgZ/arA== Date: Sun, 28 Apr 2024 09:40:33 +0300 From: Mike Rapoport To: Wei Yang Cc: akpm@linux-foundation.org, linux-mm@kvack.org Subject: Re: [Patch v2 2/8] memblock tests: add memblock_reserve_many_may_conflict_check() Message-ID: References: <20240425071929.18004-1-richard.weiyang@gmail.com> <20240425071929.18004-3-richard.weiyang@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240425071929.18004-3-richard.weiyang@gmail.com> X-Stat-Signature: kkthmooa8tn91qfh4urfqjrryqmqdef1 X-Rspam-User: X-Rspamd-Queue-Id: D8A92180006 X-Rspamd-Server: rspam05 X-HE-Tag: 1714286518-554363 X-HE-Meta: U2FsdGVkX19qEljDvUggc9qAAi18Jjdu0Rhw2p6NuaoXGj8pQ1NsuyecGuUQ0DjrcSBXzd7Ibj6h9gi+aJpdvN/SIbUakXboy2FOFq4ZlTMT8zPt3Xc0Mz2gKkJ2WEb5d2pnQWyFNRvu3UWect7xnigg15iDrVWdUItqBwzkkrBl8wIu4WvcTPhmZsl4lYcc+Dx/n/7P90Pswzk+M9YCWdxI277v/8oxyehlv7YPWPaMj1j00TrgoMuJoNQTKGXxIL9It5QDw979Jmp3kXGITEw7DAy5NFCDaKbSIRc9LkY228/qUe2C6oJwAIyjqGf15Svzf4ehdybydL8YIjWxmZtl1RSuxFE2dd9Ziy76o2UC7XkikJH/Z5d3HfkhTqDJimy1JgEMTrfDkJGHyreq/CC5a6UZoR8O3K8TYNLy45hmXsJOD3vGpAxW9bwHhiEumJGHfwjZsEJY2ujh1/CppDow8Vzd61vlhASqmLoyDWf2uC0edCG0c4YH295SIkZ6Ew5V6sZB03tibc1V41rdTioUhhohH5E2THsS8Y/r+YLQFRNpeJyvPPyPSBnudB6Wh34MYUWT4Qiab0VT3K6a3J/tmyJIuu8+AOeytQJ79iAYCCcOxcTwqZG0vuCdeDO+MmS1C28W07k/YfIh8SG/u6eJfxwYwYpN/wagENviS3QUlBZ7jNAPMjfji5GFgT5AS/ar5jLZfjwaUbg9GtRKhNy7qrC5gZh25n+/4UPdHu9GEPF6ad8xXlKqmsbYLPHupo10qy7UPfXVe+TYsT62hpvz0c5qKXZwkBPBGyiuNIoj7PxU6L8zqQudpbaOTyKHf41a/0sHrHI/dYBbpjf3AdowVHopaY4G8/k3wfBOvPQRmYEsJBs2MBPxLA+qkZGcm482OumBr053qNDkzGjuiiyQ6nVr8E2WMfHrTZP9bMDUTIZH7/szmWt+THYLtfuulAqh8TUCt8JukIGiYff ZCVsF0dq WdI1ENxENzr2pScFzIXXmd9qbr6I/ddu4Ru18i8U9nMPFs6W+mKR3qXGbeDKh6YE8XwbkBTKf1Icmh9bn1BXmSFq339KTIWJmZH1SmvjxNcwfSIp5ZG3YN657vjXn2OVkgJPFZDg4+UZ89Oc/QGmTHf/qMW2FS6LAtjzJRsZOs6OGN9ZdM4PH4A6wdzzWmLLUxhbF5xqXOky6kvVtzQ4FhREoH5v9bi0Dm79DeejiyU91flCvbeHG+L1UaxOLmu+5flErt4EBOgK15Y93H6v9CzXtvbm2uo/TGnoS 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 25, 2024 at 07:19:23AM +0000, Wei Yang wrote: > This may trigger the case fixed by commit 48c3b583bbdd ("mm/memblock: > fix overlapping allocation when doubling reserved array"). > > This is done by adding the 129th reserve region into memblock.memory. If > memblock_double_array() use this reserve region as new array, it fails. > > Signed-off-by: Wei Yang > --- > tools/testing/memblock/tests/basic_api.c | 123 +++++++++++++++++++++++ > tools/testing/memblock/tests/common.c | 4 +- > tools/testing/memblock/tests/common.h | 1 + > 3 files changed, 126 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c > index 1ae62272867a..748950e02589 100644 > --- a/tools/testing/memblock/tests/basic_api.c > +++ b/tools/testing/memblock/tests/basic_api.c > @@ -991,6 +991,128 @@ static int memblock_reserve_many_check(void) > return 0; > } > > +/* Keep the gap so these memory region will not be merged. */ The gap where? What regions should not be merged? Also please add a comment with the test description > +#define MEMORY_BASE_OFFSET(idx, offset) ((offset) + (MEM_SIZE * 2) * (idx)) > +static int memblock_reserve_many_may_conflict_check(void) > +{ > + int i, skip; > + void *orig_region; > + struct region r = { > + .base = SZ_16K, > + .size = SZ_16K, > + }; > + phys_addr_t new_reserved_regions_size; > + > + /* > + * 0 1 129 > + * +---+ +---+ +---+ > + * |32K| |32K| .. |32K| > + * +---+ +---+ +---+ > + * > + * Pre-allocate the range for 129 memory block + one range for double > + * memblock.reserved.regions at idx 0. > + * See commit 48c3b583bbdd ("mm/memblock: fix overlapping allocation > + * when doubling reserved array") Sorry, but I'm failing to parse it > + */ > + dummy_physical_memory_init(); > + phys_addr_t memory_base = dummy_physical_memory_base(); > + phys_addr_t offset = PAGE_ALIGN(memory_base); > + > + PREFIX_PUSH(); > + > + /* Reserve the 129th memory block for all possible positions*/ > + for (skip = 1; skip <= INIT_MEMBLOCK_REGIONS + 1; skip++) { > + reset_memblock_regions(); > + memblock_allow_resize(); > + > + reset_memblock_attributes(); > + /* Add a valid memory region used by double_array(). */ > + memblock_add(MEMORY_BASE_OFFSET(0, offset), MEM_SIZE); > + /* > + * Add a memory region which will be reserved as 129th memory > + * region. This is not expected to be used by double_array(). > + */ > + memblock_add(MEMORY_BASE_OFFSET(skip, offset), MEM_SIZE); > + > + for (i = 1; i <= INIT_MEMBLOCK_REGIONS + 1; i++) { > + if (i == skip) > + continue; > + > + /* Reserve some fakes memory region to fulfill the memblock. */ > + memblock_reserve(MEMORY_BASE_OFFSET(i, offset), MEM_SIZE); > + > + if (i < skip) { > + ASSERT_EQ(memblock.reserved.cnt, i); > + ASSERT_EQ(memblock.reserved.total_size, i * MEM_SIZE); > + } else { > + ASSERT_EQ(memblock.reserved.cnt, i - 1); > + ASSERT_EQ(memblock.reserved.total_size, (i - 1) * MEM_SIZE); > + } > + } > + > + orig_region = memblock.reserved.regions; > + > + /* This reserve the 129 memory_region, and makes it double array. */ > + memblock_reserve(MEMORY_BASE_OFFSET(skip, offset), MEM_SIZE); > + > + /* > + * This is the memory region size used by the doubled reserved.regions, > + * and it has been reserved due to it has been used. The size is used to > + * calculate the total_size that the memblock.reserved have now. > + */ > + new_reserved_regions_size = PAGE_ALIGN((INIT_MEMBLOCK_REGIONS * 2) * > + sizeof(struct memblock_region)); > + /* > + * The double_array() will find a free memory region as the new > + * reserved.regions, and the used memory region will be reserved, so > + * there will be one more region exist in the reserved memblock. And the > + * one more reserved region's size is new_reserved_regions_size. > + */ > + ASSERT_EQ(memblock.reserved.cnt, INIT_MEMBLOCK_REGIONS + 2); > + ASSERT_EQ(memblock.reserved.total_size, (INIT_MEMBLOCK_REGIONS + 1) * MEM_SIZE + > + new_reserved_regions_size); > + ASSERT_EQ(memblock.reserved.max, INIT_MEMBLOCK_REGIONS * 2); > + > + /* > + * The first reserved region is allocated for double array > + * with the size of new_reserved_regions_size and the base to be > + * MEMORY_BASE_OFFSET(0, offset) + SZ_32K - new_reserved_regions_size > + */ > + ASSERT_EQ(memblock.reserved.regions[0].base + memblock.reserved.regions[0].size, > + MEMORY_BASE_OFFSET(0, offset) + SZ_32K); > + ASSERT_EQ(memblock.reserved.regions[0].size, new_reserved_regions_size); > + > + /* > + * Now memblock_double_array() works fine. Let's check after the > + * double_array(), the memblock_reserve() still works as normal. > + */ > + memblock_reserve(r.base, r.size); > + ASSERT_EQ(memblock.reserved.regions[0].base, r.base); > + ASSERT_EQ(memblock.reserved.regions[0].size, r.size); > + > + ASSERT_EQ(memblock.reserved.cnt, INIT_MEMBLOCK_REGIONS + 3); > + ASSERT_EQ(memblock.reserved.total_size, (INIT_MEMBLOCK_REGIONS + 1) * MEM_SIZE + > + new_reserved_regions_size + > + r.size); > + ASSERT_EQ(memblock.reserved.max, INIT_MEMBLOCK_REGIONS * 2); > + > + /* > + * The current reserved.regions is occupying a range of memory that > + * allocated from dummy_physical_memory_init(). After free the memory, > + * we must not use it. So restore the origin memory region to make sure > + * the tests can run as normal and not affected by the double array. > + */ > + memblock.reserved.regions = orig_region; > + memblock.reserved.cnt = INIT_MEMBLOCK_RESERVED_REGIONS; > + } > + > + dummy_physical_memory_cleanup(); > + > + test_pass_pop(); > + > + return 0; > +} > + > static int memblock_reserve_checks(void) > { > prefix_reset(); > @@ -1006,6 +1128,7 @@ static int memblock_reserve_checks(void) > memblock_reserve_between_check(); > memblock_reserve_near_max_check(); > memblock_reserve_many_check(); > + memblock_reserve_many_may_conflict_check(); > > prefix_pop(); > > diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c > index c2c569f12178..5633ffc5aaa7 100644 > --- a/tools/testing/memblock/tests/common.c > +++ b/tools/testing/memblock/tests/common.c > @@ -61,7 +61,7 @@ void reset_memblock_attributes(void) > > static inline void fill_memblock(void) > { > - memset(memory_block.base, 1, MEM_SIZE); > + memset(memory_block.base, 1, MEM_ALLOC_SIZE); I believe PHYS_MEM_SIZE is a better name. > } > > void setup_memblock(void) > @@ -103,7 +103,7 @@ void setup_numa_memblock(const unsigned int node_fracs[]) > > void dummy_physical_memory_init(void) > { > - memory_block.base = malloc(MEM_SIZE); > + memory_block.base = malloc(MEM_ALLOC_SIZE); > assert(memory_block.base); > fill_memblock(); > } > diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h > index b5ec59aa62d7..741d57315ba6 100644 > --- a/tools/testing/memblock/tests/common.h > +++ b/tools/testing/memblock/tests/common.h > @@ -12,6 +12,7 @@ > #include <../selftests/kselftest.h> > > #define MEM_SIZE SZ_32K > +#define MEM_ALLOC_SIZE SZ_16M Do we really need 16M? Wouldn't one or two suffice? > #define NUMA_NODES 8 > > #define INIT_MEMBLOCK_REGIONS 128 > -- > 2.34.1 > -- Sincerely yours, Mike.