Re: [Patch v2 2/8] memblock tests: add memblock_reserve_many_may_conflict_check()

[Wei Yang <richard.weiyang@gmail.com>]

On Sun, Apr 28, 2024 at 09:40:33AM +0300, Mike Rapoport wrote:
>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 <richard.weiyang@gmail.com>
>> ---
>>  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
>

Sorry for missed this one. You mean how this case is triggered?

Suppose current memblock looks like this:

                 0        1    
memblock.memory  +---+    +---+
                 |32K|    |32K|
                 +---+    +---+

                 0                2          128
memblock.reserved+---+            +---+      +---+
                 |32K|            |32K|  ..  |32K|
                 +---+            +---+      +---+
                          ^
                          |
Now we want to reserve range 1 here. Since there are 128 blocks in
memblock.reserved, it will double array. Now memblock_double_array() will
"find" available range in memblock.memory.

Since we set top down, it will find memblock.memory.regions[1]. This conflict
the range we want to reserve.

-- 
Wei Yang
Help you, Help me