On Wed, Apr 17, 2024 at 08:51:14AM +0300, Mike Rapoport wrote: >On Tue, Apr 16, 2024 at 12:55:31PM +0000, Wei Yang wrote: >> On Mon, Apr 15, 2024 at 06:19:42PM +0300, Mike Rapoport wrote: >> >On Sun, Apr 14, 2024 at 12:45:27AM +0000, Wei Yang wrote: >> >> After previous change, we may double the array based on the position of >> >> the new range. >> >> >> >> Let's make sure the 129th memory block would double the size correctly >> >> at all possible position. >> > >> >Rather than rewrite an existing test, just add a new one. >> >> Ok, will add a new one for this. >> >> >Besides, it would be more interesting to test additions to >> >memblock.reserved and a mix of memblock_add() and memblock_reserve() that >> >will require resizing the memblock arrays. >> >> I don't get this very clearly. Would you mind give more hint? > >There is memblock_reserve_many_check() that verifies that memblock.reserved >is properly resized. I think it's better to add test that adds 129th block >at multiple locations to memblock.reserved. > I write a draft as below, is this what you expect? diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c index f1569ebb9872..d2b8114921f9 100644 --- a/tools/testing/memblock/tests/basic_api.c +++ b/tools/testing/memblock/tests/basic_api.c @@ -912,84 +912,94 @@ static int memblock_reserve_near_max_check(void) * memblock.memory.max, find a new valid memory as * reserved.regions. */ +/* Keep the gap so these memory region will not be merged. */ +#define MEMORY_BASE(idx) (SZ_128K + (MEM_SIZE * 2) * (idx)) static int memblock_reserve_many_check(void) { - int i; + int i, skip; void *orig_region; struct region r = { .base = SZ_16K, .size = SZ_16K, }; - phys_addr_t memory_base = SZ_128K; phys_addr_t new_reserved_regions_size; PREFIX_PUSH(); - reset_memblock_regions(); - memblock_allow_resize(); + /* Reserve the 129th memory block for all possible positions*/ + for (skip = 0; skip < INIT_MEMBLOCK_REGIONS + 1; skip++) + { + reset_memblock_regions(); + memblock_allow_resize(); - /* Add a valid memory region used by double_array(). */ - dummy_physical_memory_init(); - memblock_add(dummy_physical_memory_base(), MEM_SIZE); + /* Add a valid memory region used by double_array(). */ + dummy_physical_memory_init(); + memblock_add(dummy_physical_memory_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); + for (i = 0; i < INIT_MEMBLOCK_REGIONS + 1; i++) { + if (i == skip) + continue; - ASSERT_EQ(memblock.reserved.cnt, i + 1); - ASSERT_EQ(memblock.reserved.total_size, (i + 1) * MEM_SIZE); + /* Reserve some fakes memory region to fulfill the memblock. */ + memblock_reserve(MEMORY_BASE(i), MEM_SIZE); - /* Keep the gap so these memory region will not be merged. */ - memory_base += MEM_SIZE * 2; - } + if (i < skip) { + ASSERT_EQ(memblock.reserved.cnt, i + 1); + ASSERT_EQ(memblock.reserved.total_size, (i + 1) * MEM_SIZE); + } else { + ASSERT_EQ(memblock.reserved.cnt, i); + ASSERT_EQ(memblock.reserved.total_size, i * MEM_SIZE); + } + } - 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 + 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); - - /* - * 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); - - 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_RESERVED_REGIONS; + orig_region = memblock.reserved.regions; + + /* This reserve the 129 memory_region, and makes it double array. */ + memblock_reserve(MEMORY_BASE(skip), 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); + + /* + * 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); + + 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_RESERVED_REGIONS; + } test_pass_pop(); -- 2.34.1 >> -- >> Wei Yang >> Help you, Help me > >-- >Sincerely yours, >Mike. -- Wei Yang Help you, Help me