Messages in this thread | | | Date | Thu, 1 Sep 2022 16:43:40 +0800 | Subject | Re: [PATCH 2/3] memblock test: Add test to memblock_reserve() 129th region | From | "Huang, Shaoqin" <> |
| |
On 9/1/2022 4:08 PM, Mike Rapoport wrote: > On Tue, Aug 30, 2022 at 09:49:18AM +0800, shaoqin.huang@intel.com wrote: >> From: Shaoqin Huang <shaoqin.huang@intel.com> >> >> Reserve 129th region in the memblock, and this will trigger the >> memblock_double_array() function, this needs valid memory regions. So >> using dummy_physical_memory_init() to allocate a valid memory region. >> At the same time, reserve 128 faked memory region, and make sure these >> reserved region not intersect with the valid memory region. So >> memblock_double_array() will choose the valid memory region, and it will >> success. >> >> Also need to restore the reserved.regions after memblock_double_array(), >> to make sure the subsequent tests can run as normal. >> >> Signed-off-by: Shaoqin Huang <shaoqin.huang@intel.com> >> --- >> tools/testing/memblock/tests/basic_api.c | 87 ++++++++++++++++++++++++ >> 1 file changed, 87 insertions(+) >> >> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c >> index c8e201156cdc..d8defc9866cb 100644 >> --- a/tools/testing/memblock/tests/basic_api.c >> +++ b/tools/testing/memblock/tests/basic_api.c >> @@ -686,6 +686,92 @@ static int memblock_reserve_twice_check(void) >> return 0; >> } >> >> +/* >> + * A test that tries to reserve the 129th memory block. >> + * Expect to trigger memblock_double_array() to double the >> + * memblock.memory.max, find a new valid memory as >> + * reserved.regions. >> + */ >> +static int memblock_reserve_many_check(void) >> +{ >> + int i; >> + void *orig_region; >> + struct region r = { >> + .base = SZ_16K, >> + .size = MEM_SIZE, >> + }; >> + phys_addr_t memory_base = SZ_128K; >> + phys_addr_t new_reserved_regions_size; >> + >> + PREFIX_PUSH(); >> + >> + reset_memblock_regions(); >> + memblock_allow_resize(); >> + >> + /* Add a valid memory region used by double_array(). */ >> + dummy_physical_memory_init(); >> + memblock_add((phys_addr_t)get_memory_block_base(), MEM_SIZE); >> + >> + for (i = 0; i < INIT_MEMBLOCK_REGIONS; i++) { >> + /* Reserve some fakes memory region to fulfill the memblock. */ >> + memblock_reserve(memory_base, MEM_SIZE); >> + >> + ASSERT_EQ(memblock.reserved.cnt, i + 1); >> + ASSERT_EQ(memblock.reserved.total_size, (i + 1) * MEM_SIZE); >> + >> + /* Keep the gap so these memory region will not be merged. */ >> + memory_base += MEM_SIZE * 2; >> + } >> + >> + orig_region = memblock.reserved.regions; >> + >> + /* This reserve the 129 memory_region, and makes it double array. */ >> + memblock_reserve(memory_base, 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 + 1 + 1); > > +2 would be fine ^ >
Yes. It actually is +2. first +1 is the 129th region, second +1 is the reserved region used by double_array().
>> + 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); >> + > > Can you please elaborate what does the below sequence check? >
After the double_array(), we can reserve more memory region. The below is aimed to check it can reserve more. So this reserve a memory region with small base which will be put at the first reserved.regions, and I checked if it will be reserved ok.
>> + /* The base is very small, so it should be insert to the first region. */ >> + 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 + 2 + 1); >> + ASSERT_EQ(memblock.reserved.total_size, (INIT_MEMBLOCK_REGIONS + 2) * MEM_SIZE + >> + new_reserved_regions_size); >> + ASSERT_EQ(memblock.reserved.max, INIT_MEMBLOCK_REGIONS * 2); >> + >> + dummy_physical_memory_cleanup(); >> + >> + /* >> + * 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_REGIONS; >> + >> + test_pass_pop(); >> + >> + return 0; >> +} >> + >> static int memblock_reserve_checks(void) >> { >> prefix_reset(); >> @@ -698,6 +784,7 @@ static int memblock_reserve_checks(void) >> memblock_reserve_overlap_bottom_check(); >> memblock_reserve_within_check(); >> memblock_reserve_twice_check(); >> + memblock_reserve_many_check(); >> >> prefix_pop(); >> >> -- >> 2.34.1 >> >
| |