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

[Mike Rapoport <rppt@kernel.org>]

On Tue, Apr 30, 2024 at 01:49:05AM +0000, Wei Yang wrote:
> 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.

Please include something like this explanation in the test description
 
> -- 
> Wei Yang
> Help you, Help me

-- 
Sincerely yours,
Mike.