[PATCH v2 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region

[PATCH v2 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region

[shaoqin.huang]

From: Shaoqin Huang <shaoqin.huang@intel.com>

These tests is aimed for testing the memblock_double_array() can work normal. It
will using the dummy_physical_memory_init() to add the valid memory region into
the memblock.memory, and this memory region will be choosed when
memblock_double_array() to allocate the new memory region to double the regions.
Thus the new memory.regions or reserved.regions will occupy the valid memory
region, and the memory.max and reserved.max also being doubled. Check all of
these changed stuff, to make sure it actually success.

Changelog:
----------
v2:
  - Modify the get_memory_block_base() to dummy_physical_memory_base().
  - memory_add() the memory which is allocated from dummy_physical_memory_init()
  instead of some faked memory.
  - Add more comments to illustrate the test process.
  - Add a function dummy_physical_memory_cleanup_many() to free multiple memory
  which is allocated from dummy_physical_memory_init().

Shaoqin Huang (3):
  memblock test: Add test to memblock_add() 129th region
  memblock test: Add test to memblock_reserve() 129th region
  memblock test: Update TODO list

 tools/testing/memblock/TODO              |  11 +-
 tools/testing/memblock/tests/basic_api.c | 187 +++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |  15 +-
 tools/testing/memblock/tests/common.h    |   4 +
 4 files changed, 206 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] memblock test: Add test to memblock_add() 129th region

[shaoqin.huang]

From: Shaoqin Huang <shaoqin.huang@intel.com>

Add 129th region into the memblock, and this will trigger the
memblock_double_array() function, this needs valid memory regions. So
using dummy_physical_memory_init() to allocate some valid memory region,
and add it into the memblock. It make sure the memblock_double_array()
will always choose the valid memory region that is allocated by the
dummy_physical_memory_init(). So memblock_double_array() must success.

Another thing should be done is to restore the memory.regions after
memblock_double_array(), due to now the memory.regions is pointing to a
memory region allocated by dummy_physical_memory_init(). And it will
affect the subsequent tests if we don't restore the memory region. So
simply record the origin region, and restore it after the test.

Signed-off-by: Shaoqin Huang <shaoqin.huang@intel.com>
---
 tools/testing/memblock/tests/basic_api.c | 91 ++++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    | 15 +++-
 tools/testing/memblock/tests/common.h    |  4 ++
 3 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index a13a57ba0815..b9877344d3a1 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -423,6 +423,96 @@ static int memblock_add_near_max_check(void)
 	return 0;
 }
 
+/*
+ * A test that trying to add the 129th memory block.
+ * Expect to trigger memblock_double_array() to double the
+ * memblock.memory.max, find a new valid memory as
+ * memory.regions.
+ */
+static int memblock_add_many_check(void)
+{
+	int i;
+	void *orig_region;
+	struct region r = {
+		.base = SZ_16K,
+		.size = MEM_SIZE,
+	};
+	/* Record these allocated memory, they will be free at the end. */
+	phys_addr_t base[INIT_MEMBLOCK_REGIONS + 1];
+
+	PREFIX_PUSH();
+
+	reset_memblock_regions();
+	memblock_allow_resize();
+
+	for (i = 0; i < INIT_MEMBLOCK_REGIONS; i++) {
+		/* Add some memory region to fulfill the memblock. */
+		dummy_physical_memory_init();
+		base[i] = dummy_physical_memory_base();
+		memblock_add(base[i], MEM_SIZE);
+
+		ASSERT_EQ(memblock.memory.cnt, i + 1);
+		ASSERT_EQ(memblock.memory.total_size, (i + 1) * MEM_SIZE);
+	}
+
+	orig_region = memblock.memory.regions;
+
+	/*
+	 * This adds the 129 memory_region, and makes it double array. Now
+	 * MEM_SIZE is 16K, which is enough, the doubled array will occupy 8K
+	 * memory region, so it must success.
+	 */
+	dummy_physical_memory_init();
+	base[i] = dummy_physical_memory_base();
+	memblock_add(base[i], MEM_SIZE);
+
+	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 1);
+	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 1) * MEM_SIZE);
+	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
+
+	ASSERT_EQ(memblock.reserved.cnt, 1);
+	/* This is the size used by new memory.regions. Check it. */
+	ASSERT_EQ(memblock.reserved.total_size, PAGE_ALIGN(INIT_MEMBLOCK_REGIONS * 2 *
+						sizeof(struct memblock_region)));
+
+	/*
+	 * After double array, we want to make sure the memblock.memory.regions
+	 * is actually on a valid memory, so we try to add a memory region which
+	 * the base is very small, it should be insert to the first region. And
+	 * the memory.cnt and memory.total_size will both be changed.
+	 * Let's check it.
+	 */
+	memblock_add(r.base, r.size);
+	ASSERT_EQ(memblock.memory.regions[0].base, r.base);
+	ASSERT_EQ(memblock.memory.regions[0].size, r.size);
+
+	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 2);
+	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 2) * MEM_SIZE);
+	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
+
+	/*
+	 * Due to we dummy_physical_memory_init() many memory region in this
+	 * test, we need to free it. Instead of expose the memory_block and
+	 * directly modify it's base, we pass an array which record all the
+	 * memory base that we allocated to this function, and let it to do the
+	 * clean job.
+	 */
+	dummy_physical_memory_cleanup_many(base, INIT_MEMBLOCK_REGIONS + 1);
+
+	/*
+	 * The current memory.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.memory.regions = orig_region;
+	memblock.memory.cnt = INIT_MEMBLOCK_REGIONS;
+
+	test_pass_pop();
+
+	return 0;
+}
+
 static int memblock_add_checks(void)
 {
 	prefix_reset();
@@ -438,6 +528,7 @@ static int memblock_add_checks(void)
 	memblock_add_twice_check();
 	memblock_add_between_check();
 	memblock_add_near_max_check();
+	memblock_add_many_check();
 
 	prefix_pop();
 
diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
index eec6901081af..1fb347c5c099 100644
--- a/tools/testing/memblock/tests/common.c
+++ b/tools/testing/memblock/tests/common.c
@@ -5,8 +5,6 @@
 #include <linux/memory_hotplug.h>
 #include <linux/build_bug.h>
 
-#define INIT_MEMBLOCK_REGIONS			128
-#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
 #define PREFIXES_MAX				15
 #define DELIM					": "
 
@@ -84,6 +82,19 @@ void dummy_physical_memory_cleanup(void)
 	free(memory_block.base);
 }
 
+void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt)
+{
+	for (int i = 0; i < cnt; i++) {
+		memory_block.base = (void *)base[i];
+		dummy_physical_memory_cleanup();
+	}
+}
+
+phys_addr_t dummy_physical_memory_base(void)
+{
+	return (phys_addr_t)memory_block.base;
+}
+
 static void usage(const char *prog)
 {
 	BUILD_BUG_ON(ARRAY_SIZE(help_opts) != ARRAY_SIZE(long_opts) - 1);
diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
index 78128e109a95..310f0be2b2a2 100644
--- a/tools/testing/memblock/tests/common.h
+++ b/tools/testing/memblock/tests/common.h
@@ -11,6 +11,8 @@
 #include <../selftests/kselftest.h>
 
 #define MEM_SIZE SZ_16K
+#define INIT_MEMBLOCK_REGIONS			128
+#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
 
 enum test_flags {
 	/* No special request. */
@@ -104,6 +106,8 @@ void reset_memblock_attributes(void);
 void setup_memblock(void);
 void dummy_physical_memory_init(void);
 void dummy_physical_memory_cleanup(void);
+void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt);
+phys_addr_t dummy_physical_memory_base(void);
 void parse_args(int argc, char **argv);
 
 void test_fail(void);
-- 
2.34.1

[PATCH v2 2/3] memblock test: Add test to memblock_reserve() 129th region

[shaoqin.huang]

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 | 96 ++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index b9877344d3a1..0b23a656c145 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -890,6 +890,101 @@ static int memblock_reserve_near_max_check(void)
 	return 0;
 }
 
+/*
+ * A test that trying 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(). Now MEM_SIZE is
+	 * 16K, which is enough. The new array will only occupy 8K.
+	 */
+	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);
+
+		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 + 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);
+
+	/*
+	 * After double array, we want to make sure the memblock.reserved.regions
+	 * is actually on a valid memory, so we try to reserve a memory region which
+	 * the base is very small, it should be insert to the first region. And
+	 * the reserved.cnt and reserved.total_size will both be changed.
+	 * Let's check it.
+	 */
+	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 + 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();
@@ -904,6 +999,7 @@ static int memblock_reserve_checks(void)
 	memblock_reserve_twice_check();
 	memblock_reserve_between_check();
 	memblock_reserve_near_max_check();
+	memblock_reserve_many_check();
 
 	prefix_pop();
 
-- 
2.34.1

[PATCH v2 3/3] memblock test: Update TODO list

[shaoqin.huang]

From: Shaoqin Huang <shaoqin.huang@intel.com>

Remove the completed items from TODO list.

Signed-off-by: Shaoqin Huang <shaoqin.huang@intel.com>
---
 tools/testing/memblock/TODO | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/tools/testing/memblock/TODO b/tools/testing/memblock/TODO
index 33044c634ea7..503cc96fcdc3 100644
--- a/tools/testing/memblock/TODO
+++ b/tools/testing/memblock/TODO
@@ -1,17 +1,10 @@
 TODO
 =====
 
-1. Add tests trying to memblock_add() or memblock_reserve() 129th region.
-   This will trigger memblock_double_array(), make sure it succeeds.
-   *Important:* These tests require valid memory ranges, use dummy physical
-                memory block from common.c to implement them. It is also very
-                likely that the current MEM_SIZE won't be enough for these
-                test cases. Use realloc to adjust the size accordingly.
-
-2. Add test cases using this functions (implement them for both directions):
+1. Add test cases using this functions (implement them for both directions):
    + memblock_alloc_raw()
    + memblock_alloc_exact_nid_raw()
    + memblock_alloc_try_nid_raw()
 
-3. Add tests for memblock_alloc_node() to check if the correct NUMA node is set
+2. Add tests for memblock_alloc_node() to check if the correct NUMA node is set
    for the new region
-- 
2.34.1

Re: [PATCH v2 1/3] memblock test: Add test to memblock_add() 129th region

[Mike Rapoport]

On Fri, Sep 02, 2022 at 10:39:59AM +0800, shaoqin.huang@intel.com wrote:
> From: Shaoqin Huang <shaoqin.huang@intel.com>
> 
> Add 129th region into the memblock, and this will trigger the
> memblock_double_array() function, this needs valid memory regions. So
> using dummy_physical_memory_init() to allocate some valid memory region,
> and add it into the memblock. It make sure the memblock_double_array()
> will always choose the valid memory region that is allocated by the
> dummy_physical_memory_init(). So memblock_double_array() must success.
> 
> Another thing should be done is to restore the memory.regions after
> memblock_double_array(), due to now the memory.regions is pointing to a
> memory region allocated by dummy_physical_memory_init(). And it will
> affect the subsequent tests if we don't restore the memory region. So
> simply record the origin region, and restore it after the test.
> 
> Signed-off-by: Shaoqin Huang <shaoqin.huang@intel.com>
> ---
>  tools/testing/memblock/tests/basic_api.c | 91 ++++++++++++++++++++++++
>  tools/testing/memblock/tests/common.c    | 15 +++-
>  tools/testing/memblock/tests/common.h    |  4 ++
>  3 files changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
> index a13a57ba0815..b9877344d3a1 100644
> --- a/tools/testing/memblock/tests/basic_api.c
> +++ b/tools/testing/memblock/tests/basic_api.c
> @@ -423,6 +423,96 @@ static int memblock_add_near_max_check(void)
>  	return 0;
>  }
>  
> +/*
> + * A test that trying to add the 129th memory block.
> + * Expect to trigger memblock_double_array() to double the
> + * memblock.memory.max, find a new valid memory as
> + * memory.regions.
> + */
> +static int memblock_add_many_check(void)
> +{
> +	int i;
> +	void *orig_region;
> +	struct region r = {
> +		.base = SZ_16K,
> +		.size = MEM_SIZE,
> +	};
> +	/* Record these allocated memory, they will be free at the end. */
> +	phys_addr_t base[INIT_MEMBLOCK_REGIONS + 1];
> +
> +	PREFIX_PUSH();
> +
> +	reset_memblock_regions();
> +	memblock_allow_resize();
> +
> +	for (i = 0; i < INIT_MEMBLOCK_REGIONS; i++) {
> +		/* Add some memory region to fulfill the memblock. */
> +		dummy_physical_memory_init();
> +		base[i] = dummy_physical_memory_base();
> +		memblock_add(base[i], MEM_SIZE);

Looks like we are going in rounds.
The simulated physical memory is what dummy_physical_memory_init()
allocates. Every memblock_add() may take a range from that "physical
memory" and register it with memblock. There is no need to allocate new
chunk for every memblock_add(), just make sure that
dummy_physical_memory_init() allocates enough memory.

> +
> +		ASSERT_EQ(memblock.memory.cnt, i + 1);
> +		ASSERT_EQ(memblock.memory.total_size, (i + 1) * MEM_SIZE);
> +	}
> +
> +	orig_region = memblock.memory.regions;
> +
> +	/*
> +	 * This adds the 129 memory_region, and makes it double array. Now
> +	 * MEM_SIZE is 16K, which is enough, the doubled array will occupy 8K
> +	 * memory region, so it must success.
> +	 */
> +	dummy_physical_memory_init();
> +	base[i] = dummy_physical_memory_base();
> +	memblock_add(base[i], MEM_SIZE);
> +
> +	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 1);
> +	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 1) * MEM_SIZE);
> +	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
> +
> +	ASSERT_EQ(memblock.reserved.cnt, 1);
> +	/* This is the size used by new memory.regions. Check it. */
> +	ASSERT_EQ(memblock.reserved.total_size, PAGE_ALIGN(INIT_MEMBLOCK_REGIONS * 2 *
> +						sizeof(struct memblock_region)));
> +
> +	/*
> +	 * After double array, we want to make sure the memblock.memory.regions
> +	 * is actually on a valid memory, so we try to add a memory region which

What do you mean by "actually on a valid memory"? How would
memblock_double_array() succeed otherwise?

> +	 * the base is very small, it should be insert to the first region. And
> +	 * the memory.cnt and memory.total_size will both be changed.
> +	 * Let's check it.
> +	 */
> +	memblock_add(r.base, r.size);
> +	ASSERT_EQ(memblock.memory.regions[0].base, r.base);
> +	ASSERT_EQ(memblock.memory.regions[0].size, r.size);
> +
> +	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 2);
> +	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 2) * MEM_SIZE);
> +	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
> +
> +	/*
> +	 * Due to we dummy_physical_memory_init() many memory region in this
> +	 * test, we need to free it. Instead of expose the memory_block and
> +	 * directly modify it's base, we pass an array which record all the
> +	 * memory base that we allocated to this function, and let it to do the
> +	 * clean job.
> +	 */
> +	dummy_physical_memory_cleanup_many(base, INIT_MEMBLOCK_REGIONS + 1);
> +
> +	/*
> +	 * The current memory.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.memory.regions = orig_region;
> +	memblock.memory.cnt = INIT_MEMBLOCK_REGIONS;
> +
> +	test_pass_pop();
> +
> +	return 0;
> +}
> +
>  static int memblock_add_checks(void)
>  {
>  	prefix_reset();
> @@ -438,6 +528,7 @@ static int memblock_add_checks(void)
>  	memblock_add_twice_check();
>  	memblock_add_between_check();
>  	memblock_add_near_max_check();
> +	memblock_add_many_check();
>  
>  	prefix_pop();
>  
> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
> index eec6901081af..1fb347c5c099 100644
> --- a/tools/testing/memblock/tests/common.c
> +++ b/tools/testing/memblock/tests/common.c
> @@ -5,8 +5,6 @@
>  #include <linux/memory_hotplug.h>
>  #include <linux/build_bug.h>
>  
> -#define INIT_MEMBLOCK_REGIONS			128
> -#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
>  #define PREFIXES_MAX				15
>  #define DELIM					": "
>  
> @@ -84,6 +82,19 @@ void dummy_physical_memory_cleanup(void)
>  	free(memory_block.base);
>  }
>  
> +void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt)
> +{
> +	for (int i = 0; i < cnt; i++) {
> +		memory_block.base = (void *)base[i];
> +		dummy_physical_memory_cleanup();
> +	}
> +}
> +
> +phys_addr_t dummy_physical_memory_base(void)
> +{
> +	return (phys_addr_t)memory_block.base;
> +}
> +
>  static void usage(const char *prog)
>  {
>  	BUILD_BUG_ON(ARRAY_SIZE(help_opts) != ARRAY_SIZE(long_opts) - 1);
> diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
> index 78128e109a95..310f0be2b2a2 100644
> --- a/tools/testing/memblock/tests/common.h
> +++ b/tools/testing/memblock/tests/common.h
> @@ -11,6 +11,8 @@
>  #include <../selftests/kselftest.h>
>  
>  #define MEM_SIZE SZ_16K
> +#define INIT_MEMBLOCK_REGIONS			128
> +#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
>  
>  enum test_flags {
>  	/* No special request. */
> @@ -104,6 +106,8 @@ void reset_memblock_attributes(void);
>  void setup_memblock(void);
>  void dummy_physical_memory_init(void);
>  void dummy_physical_memory_cleanup(void);
> +void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt);
> +phys_addr_t dummy_physical_memory_base(void);
>  void parse_args(int argc, char **argv);
>  
>  void test_fail(void);
> -- 
> 2.34.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 1/3] memblock test: Add test to memblock_add() 129th region

[Huang, Shaoqin]

On 9/6/2022 8:57 PM, Mike Rapoport wrote:
> On Fri, Sep 02, 2022 at 10:39:59AM +0800, shaoqin.huang@intel.com wrote:
>> From: Shaoqin Huang <shaoqin.huang@intel.com>
>>
>> Add 129th region into the memblock, and this will trigger the
>> memblock_double_array() function, this needs valid memory regions. So
>> using dummy_physical_memory_init() to allocate some valid memory region,
>> and add it into the memblock. It make sure the memblock_double_array()
>> will always choose the valid memory region that is allocated by the
>> dummy_physical_memory_init(). So memblock_double_array() must success.
>>
>> Another thing should be done is to restore the memory.regions after
>> memblock_double_array(), due to now the memory.regions is pointing to a
>> memory region allocated by dummy_physical_memory_init(). And it will
>> affect the subsequent tests if we don't restore the memory region. So
>> simply record the origin region, and restore it after the test.
>>
>> Signed-off-by: Shaoqin Huang <shaoqin.huang@intel.com>
>> ---
>>   tools/testing/memblock/tests/basic_api.c | 91 ++++++++++++++++++++++++
>>   tools/testing/memblock/tests/common.c    | 15 +++-
>>   tools/testing/memblock/tests/common.h    |  4 ++
>>   3 files changed, 108 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
>> index a13a57ba0815..b9877344d3a1 100644
>> --- a/tools/testing/memblock/tests/basic_api.c
>> +++ b/tools/testing/memblock/tests/basic_api.c
>> @@ -423,6 +423,96 @@ static int memblock_add_near_max_check(void)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * A test that trying to add the 129th memory block.
>> + * Expect to trigger memblock_double_array() to double the
>> + * memblock.memory.max, find a new valid memory as
>> + * memory.regions.
>> + */
>> +static int memblock_add_many_check(void)
>> +{
>> +	int i;
>> +	void *orig_region;
>> +	struct region r = {
>> +		.base = SZ_16K,
>> +		.size = MEM_SIZE,
>> +	};
>> +	/* Record these allocated memory, they will be free at the end. */
>> +	phys_addr_t base[INIT_MEMBLOCK_REGIONS + 1];
>> +
>> +	PREFIX_PUSH();
>> +
>> +	reset_memblock_regions();
>> +	memblock_allow_resize();
>> +
>> +	for (i = 0; i < INIT_MEMBLOCK_REGIONS; i++) {
>> +		/* Add some memory region to fulfill the memblock. */
>> +		dummy_physical_memory_init();
>> +		base[i] = dummy_physical_memory_base();
>> +		memblock_add(base[i], MEM_SIZE);
> 
> Looks like we are going in rounds.
> The simulated physical memory is what dummy_physical_memory_init()
> allocates. Every memblock_add() may take a range from that "physical
> memory" and register it with memblock. There is no need to allocate new
> chunk for every memblock_add(), just make sure that
> dummy_physical_memory_init() allocates enough memory.
> 

Do you mean dummy_physical_memory_init() a large memory? And split it 
into 128 regions to fulfill the memory.regions. If so, we may need to 
modify the MEM_SIZE (now is 16K) to a larger size to make double_array() 
succeed due to the new memory.regions will occupy 8K memory.

>> +
>> +		ASSERT_EQ(memblock.memory.cnt, i + 1);
>> +		ASSERT_EQ(memblock.memory.total_size, (i + 1) * MEM_SIZE);
>> +	}
>> +
>> +	orig_region = memblock.memory.regions;
>> +
>> +	/*
>> +	 * This adds the 129 memory_region, and makes it double array. Now
>> +	 * MEM_SIZE is 16K, which is enough, the doubled array will occupy 8K
>> +	 * memory region, so it must success.
>> +	 */
>> +	dummy_physical_memory_init();
>> +	base[i] = dummy_physical_memory_base();
>> +	memblock_add(base[i], MEM_SIZE);
>> +
>> +	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 1);
>> +	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 1) * MEM_SIZE);
>> +	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
>> +
>> +	ASSERT_EQ(memblock.reserved.cnt, 1);
>> +	/* This is the size used by new memory.regions. Check it. */
>> +	ASSERT_EQ(memblock.reserved.total_size, PAGE_ALIGN(INIT_MEMBLOCK_REGIONS * 2 *
>> +						sizeof(struct memblock_region)));
>> +
>> +	/*
>> +	 * After double array, we want to make sure the memblock.memory.regions
>> +	 * is actually on a valid memory, so we try to add a memory region which
> 
> What do you mean by "actually on a valid memory"? How would
> memblock_double_array() succeed otherwise?
> 

The code before this line has checked the memblock_double_array() 
succeed. I just want to test after it succeed, add a new memory region 
into it will still succeed. So the "actually on a valid memory" is a 
little confused, I want to modify "After double array, we want to make 
sure the memblock.memory.regions is actually on a valid memory" to "The 
next we want to test if the memblock_add() still succeed after 
memblock_double_array()".

>> +	 * the base is very small, it should be insert to the first region. And
>> +	 * the memory.cnt and memory.total_size will both be changed.
>> +	 * Let's check it.
>> +	 */
>> +	memblock_add(r.base, r.size);
>> +	ASSERT_EQ(memblock.memory.regions[0].base, r.base);
>> +	ASSERT_EQ(memblock.memory.regions[0].size, r.size);
>> +
>> +	ASSERT_EQ(memblock.memory.cnt, INIT_MEMBLOCK_REGIONS + 2);
>> +	ASSERT_EQ(memblock.memory.total_size, (INIT_MEMBLOCK_REGIONS + 2) * MEM_SIZE);
>> +	ASSERT_EQ(memblock.memory.max, INIT_MEMBLOCK_REGIONS * 2);
>> +
>> +	/*
>> +	 * Due to we dummy_physical_memory_init() many memory region in this
>> +	 * test, we need to free it. Instead of expose the memory_block and
>> +	 * directly modify it's base, we pass an array which record all the
>> +	 * memory base that we allocated to this function, and let it to do the
>> +	 * clean job.
>> +	 */
>> +	dummy_physical_memory_cleanup_many(base, INIT_MEMBLOCK_REGIONS + 1);
>> +
>> +	/*
>> +	 * The current memory.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.memory.regions = orig_region;
>> +	memblock.memory.cnt = INIT_MEMBLOCK_REGIONS;
>> +
>> +	test_pass_pop();
>> +
>> +	return 0;
>> +}
>> +
>>   static int memblock_add_checks(void)
>>   {
>>   	prefix_reset();
>> @@ -438,6 +528,7 @@ static int memblock_add_checks(void)
>>   	memblock_add_twice_check();
>>   	memblock_add_between_check();
>>   	memblock_add_near_max_check();
>> +	memblock_add_many_check();
>>   
>>   	prefix_pop();
>>   
>> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
>> index eec6901081af..1fb347c5c099 100644
>> --- a/tools/testing/memblock/tests/common.c
>> +++ b/tools/testing/memblock/tests/common.c
>> @@ -5,8 +5,6 @@
>>   #include <linux/memory_hotplug.h>
>>   #include <linux/build_bug.h>
>>   
>> -#define INIT_MEMBLOCK_REGIONS			128
>> -#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
>>   #define PREFIXES_MAX				15
>>   #define DELIM					": "
>>   
>> @@ -84,6 +82,19 @@ void dummy_physical_memory_cleanup(void)
>>   	free(memory_block.base);
>>   }
>>   
>> +void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt)
>> +{
>> +	for (int i = 0; i < cnt; i++) {
>> +		memory_block.base = (void *)base[i];
>> +		dummy_physical_memory_cleanup();
>> +	}
>> +}
>> +
>> +phys_addr_t dummy_physical_memory_base(void)
>> +{
>> +	return (phys_addr_t)memory_block.base;
>> +}
>> +
>>   static void usage(const char *prog)
>>   {
>>   	BUILD_BUG_ON(ARRAY_SIZE(help_opts) != ARRAY_SIZE(long_opts) - 1);
>> diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
>> index 78128e109a95..310f0be2b2a2 100644
>> --- a/tools/testing/memblock/tests/common.h
>> +++ b/tools/testing/memblock/tests/common.h
>> @@ -11,6 +11,8 @@
>>   #include <../selftests/kselftest.h>
>>   
>>   #define MEM_SIZE SZ_16K
>> +#define INIT_MEMBLOCK_REGIONS			128
>> +#define INIT_MEMBLOCK_RESERVED_REGIONS		INIT_MEMBLOCK_REGIONS
>>   
>>   enum test_flags {
>>   	/* No special request. */
>> @@ -104,6 +106,8 @@ void reset_memblock_attributes(void);
>>   void setup_memblock(void);
>>   void dummy_physical_memory_init(void);
>>   void dummy_physical_memory_cleanup(void);
>> +void dummy_physical_memory_cleanup_many(phys_addr_t *base, int cnt);
>> +phys_addr_t dummy_physical_memory_base(void);
>>   void parse_args(int argc, char **argv);
>>   
>>   void test_fail(void);
>> -- 
>> 2.34.1
>>
>