linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region
@ 2022-08-30  1:49 shaoqin.huang
  2022-08-30  1:49 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: shaoqin.huang @ 2022-08-30  1:49 UTC (permalink / raw)
  To: rppt
  Cc: Shaoqin Huang, David Hildenbrand, Karolina Drobnik, linux-kernel,
	linux-mm, Rebecca Mckeever

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.

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 | 169 +++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |   7 +-
 tools/testing/memblock/tests/common.h    |   3 +
 4 files changed, 179 insertions(+), 11 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] memblock test: Add test to memblock_add() 129th region
  2022-08-30  1:49 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
@ 2022-08-30  1:49 ` shaoqin.huang
  2022-09-01  8:02   ` Mike Rapoport
  2022-08-30  1:49 ` [PATCH 2/3] memblock test: Add test to memblock_reserve() " shaoqin.huang
  2022-08-30  1:49 ` [PATCH 3/3] memblock test: Update TODO list shaoqin.huang
  2 siblings, 1 reply; 9+ messages in thread
From: shaoqin.huang @ 2022-08-30  1:49 UTC (permalink / raw)
  To: rppt
  Cc: Shaoqin Huang, Karolina Drobnik, David Hildenbrand,
	Rebecca Mckeever, linux-mm, linux-kernel

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 a valid memory region, and
fake the other memory region, so 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 | 82 ++++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |  7 +-
 tools/testing/memblock/tests/common.h    |  3 +
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index 66f46f261e66..c8e201156cdc 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -326,6 +326,87 @@ static int memblock_add_twice_check(void)
 	return 0;
 }
 
+/*
+ * A test that tries 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,
+	};
+	phys_addr_t memory_base = SZ_128K;
+
+	PREFIX_PUSH();
+
+	reset_memblock_regions();
+	memblock_allow_resize();
+
+	/*
+	 * Add one valid memory region, this will be choosed to be the memory
+	 * that new memory.regions occupied.
+	 */
+	dummy_physical_memory_init();
+	memblock_add((phys_addr_t)get_memory_block_base(), MEM_SIZE);
+
+	ASSERT_EQ(memblock.memory.cnt, 1);
+	ASSERT_EQ(memblock.memory.total_size, MEM_SIZE);
+
+	for (i = 1; i < INIT_MEMBLOCK_REGIONS; i++) {
+		/* Add some fakes memory region to fulfill the memblock. */
+		memblock_add(memory_base, MEM_SIZE);
+
+		ASSERT_EQ(memblock.memory.cnt, i + 1);
+		ASSERT_EQ(memblock.memory.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.memory.regions;
+
+	/* This adds the 129 memory_region, and makes it double array. */
+	memblock_add((phys_addr_t)memory_base, 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)));
+
+	/* The base is very small, so it should be insert to the first region. */
+	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);
+
+	dummy_physical_memory_cleanup();
+
+	/*
+	 * 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();
@@ -339,6 +420,7 @@ static int memblock_add_checks(void)
 	memblock_add_overlap_bottom_check();
 	memblock_add_within_check();
 	memblock_add_twice_check();
+	memblock_add_many_check();
 
 	prefix_pop();
 
diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
index 76a8ad818f3a..96fabd96ff31 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					": "
 
@@ -77,6 +75,11 @@ void dummy_physical_memory_cleanup(void)
 	free(memory_block.base);
 }
 
+void *get_memory_block_base(void)
+{
+	return 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 d396e5423a8e..d56af621c543 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
 
 /**
  * ASSERT_EQ():
@@ -73,6 +75,7 @@ void reset_memblock_attributes(void);
 void setup_memblock(void);
 void dummy_physical_memory_init(void);
 void dummy_physical_memory_cleanup(void);
+void *get_memory_block_base(void);
 void parse_args(int argc, char **argv);
 
 void test_fail(void);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] memblock test: Add test to memblock_reserve() 129th region
  2022-08-30  1:49 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
  2022-08-30  1:49 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang
@ 2022-08-30  1:49 ` shaoqin.huang
  2022-09-01  8:08   ` Mike Rapoport
  2022-08-30  1:49 ` [PATCH 3/3] memblock test: Update TODO list shaoqin.huang
  2 siblings, 1 reply; 9+ messages in thread
From: shaoqin.huang @ 2022-08-30  1:49 UTC (permalink / raw)
  To: rppt
  Cc: Shaoqin Huang, Karolina Drobnik, Rebecca Mckeever,
	David Hildenbrand, linux-mm, linux-kernel

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);
+	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);
+
+	/* 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] memblock test: Update TODO list
  2022-08-30  1:49 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
  2022-08-30  1:49 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang
  2022-08-30  1:49 ` [PATCH 2/3] memblock test: Add test to memblock_reserve() " shaoqin.huang
@ 2022-08-30  1:49 ` shaoqin.huang
  2 siblings, 0 replies; 9+ messages in thread
From: shaoqin.huang @ 2022-08-30  1:49 UTC (permalink / raw)
  To: rppt; +Cc: Shaoqin Huang, linux-mm, linux-kernel

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] memblock test: Add test to memblock_add() 129th region
  2022-08-30  1:49 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang
@ 2022-09-01  8:02   ` Mike Rapoport
  2022-09-01  8:35     ` Huang, Shaoqin
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Rapoport @ 2022-09-01  8:02 UTC (permalink / raw)
  To: shaoqin.huang
  Cc: Karolina Drobnik, David Hildenbrand, Rebecca Mckeever, linux-mm,
	linux-kernel

On Tue, Aug 30, 2022 at 09:49:17AM +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 a valid memory region, and
> fake the other memory region, so 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 | 82 ++++++++++++++++++++++++
>  tools/testing/memblock/tests/common.c    |  7 +-
>  tools/testing/memblock/tests/common.h    |  3 +
>  3 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
> index 66f46f261e66..c8e201156cdc 100644
> --- a/tools/testing/memblock/tests/basic_api.c
> +++ b/tools/testing/memblock/tests/basic_api.c
> @@ -326,6 +326,87 @@ static int memblock_add_twice_check(void)
>  	return 0;
>  }
>  
> +/*
> + * A test that tries 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,
> +	};
> +	phys_addr_t memory_base = SZ_128K;
> +
> +	PREFIX_PUSH();
> +
> +	reset_memblock_regions();
> +	memblock_allow_resize();
> +
> +	/*
> +	 * Add one valid memory region, this will be choosed to be the memory
> +	 * that new memory.regions occupied.
> +	 */
> +	dummy_physical_memory_init();
> +	memblock_add((phys_addr_t)get_memory_block_base(), MEM_SIZE);
> +
> +	ASSERT_EQ(memblock.memory.cnt, 1);
> +	ASSERT_EQ(memblock.memory.total_size, MEM_SIZE);
> +
> +	for (i = 1; i < INIT_MEMBLOCK_REGIONS; i++) {
> +		/* Add some fakes memory region to fulfill the memblock. */
> +		memblock_add(memory_base, MEM_SIZE);

I would rather prefer to memblock_add() ranges from the simulated memory
created in dummy_physical_memory_init(). 16K will be probably too small,
but I don't see problem with increasing MEM_SIZE.

> +
> +		ASSERT_EQ(memblock.memory.cnt, i + 1);
> +		ASSERT_EQ(memblock.memory.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.memory.regions;
> +
> +	/* This adds the 129 memory_region, and makes it double array. */
> +	memblock_add((phys_addr_t)memory_base, MEM_SIZE);

memory_base is already phys_addr_t, isn't it?

> +
> +	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)));
> +

Can you please elaborate what does the following sequence test?

> +	/* The base is very small, so it should be insert to the first region. */
> +	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);
> +
> +	dummy_physical_memory_cleanup();
> +
> +	/*
> +	 * 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();
> @@ -339,6 +420,7 @@ static int memblock_add_checks(void)
>  	memblock_add_overlap_bottom_check();
>  	memblock_add_within_check();
>  	memblock_add_twice_check();
> +	memblock_add_many_check();
>  
>  	prefix_pop();
>  
> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
> index 76a8ad818f3a..96fabd96ff31 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					": "
>  
> @@ -77,6 +75,11 @@ void dummy_physical_memory_cleanup(void)
>  	free(memory_block.base);
>  }
>  
> +void *get_memory_block_base(void)
> +{
> +	return 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 d396e5423a8e..d56af621c543 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
>  
>  /**
>   * ASSERT_EQ():
> @@ -73,6 +75,7 @@ void reset_memblock_attributes(void);
>  void setup_memblock(void);
>  void dummy_physical_memory_init(void);
>  void dummy_physical_memory_cleanup(void);
> +void *get_memory_block_base(void);

Let's make it 

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] memblock test: Add test to memblock_reserve() 129th region
  2022-08-30  1:49 ` [PATCH 2/3] memblock test: Add test to memblock_reserve() " shaoqin.huang
@ 2022-09-01  8:08   ` Mike Rapoport
  2022-09-01  8:43     ` Huang, Shaoqin
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Rapoport @ 2022-09-01  8:08 UTC (permalink / raw)
  To: shaoqin.huang
  Cc: Karolina Drobnik, Rebecca Mckeever, David Hildenbrand, linux-mm,
	linux-kernel

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 ^

> +	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?

> +	/* 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
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] memblock test: Add test to memblock_add() 129th region
  2022-09-01  8:02   ` Mike Rapoport
@ 2022-09-01  8:35     ` Huang, Shaoqin
  0 siblings, 0 replies; 9+ messages in thread
From: Huang, Shaoqin @ 2022-09-01  8:35 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Karolina Drobnik, David Hildenbrand, Rebecca Mckeever, linux-mm,
	linux-kernel



On 9/1/2022 4:02 PM, Mike Rapoport wrote:
> On Tue, Aug 30, 2022 at 09:49:17AM +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 a valid memory region, and
>> fake the other memory region, so 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 | 82 ++++++++++++++++++++++++
>>   tools/testing/memblock/tests/common.c    |  7 +-
>>   tools/testing/memblock/tests/common.h    |  3 +
>>   3 files changed, 90 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
>> index 66f46f261e66..c8e201156cdc 100644
>> --- a/tools/testing/memblock/tests/basic_api.c
>> +++ b/tools/testing/memblock/tests/basic_api.c
>> @@ -326,6 +326,87 @@ static int memblock_add_twice_check(void)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * A test that tries 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,
>> +	};
>> +	phys_addr_t memory_base = SZ_128K;
>> +
>> +	PREFIX_PUSH();
>> +
>> +	reset_memblock_regions();
>> +	memblock_allow_resize();
>> +
>> +	/*
>> +	 * Add one valid memory region, this will be choosed to be the memory
>> +	 * that new memory.regions occupied.
>> +	 */
>> +	dummy_physical_memory_init();
>> +	memblock_add((phys_addr_t)get_memory_block_base(), MEM_SIZE);
>> +
>> +	ASSERT_EQ(memblock.memory.cnt, 1);
>> +	ASSERT_EQ(memblock.memory.total_size, MEM_SIZE);
>> +
>> +	for (i = 1; i < INIT_MEMBLOCK_REGIONS; i++) {
>> +		/* Add some fakes memory region to fulfill the memblock. */
>> +		memblock_add(memory_base, MEM_SIZE);
> 
> I would rather prefer to memblock_add() ranges from the simulated memory
> created in dummy_physical_memory_init(). 16K will be probably too small,
> but I don't see problem with increasing MEM_SIZE.
> 

Yes. If we memblock_add() the memory both allocated from 
dummy_physical_memory_init(), It's no need to fake these memory regions. 
And with all valid memory region, it will always choose a valid memory 
region and double the array.

And now with calculation, 16K is enough. The doubled array will only use 
8KB, so it will success.

>> +
>> +		ASSERT_EQ(memblock.memory.cnt, i + 1);
>> +		ASSERT_EQ(memblock.memory.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.memory.regions;
>> +
>> +	/* This adds the 129 memory_region, and makes it double array. */
>> +	memblock_add((phys_addr_t)memory_base, MEM_SIZE);
> 
> memory_base is already phys_addr_t, isn't it?
> 

Thanks for notice. Will delete it.

>> +
>> +	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)));
>> +
> 
> Can you please elaborate what does the following sequence test?
> 

Before this line, all checks is to make sure the double_array have 
successfully make the size doubled and reserved a new region as the new 
memory.regions.

Below I try to add a memory region which has a small base, so it will be 
added to the first region, if it succeed. We can prove the doubled 
memory.regions has a valid memory.

I will add the commends in the next version.

>> +	/* The base is very small, so it should be insert to the first region. */
>> +	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);
>> +
>> +	dummy_physical_memory_cleanup();
>> +
>> +	/*
>> +	 * 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();
>> @@ -339,6 +420,7 @@ static int memblock_add_checks(void)
>>   	memblock_add_overlap_bottom_check();
>>   	memblock_add_within_check();
>>   	memblock_add_twice_check();
>> +	memblock_add_many_check();
>>   
>>   	prefix_pop();
>>   
>> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
>> index 76a8ad818f3a..96fabd96ff31 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					": "
>>   
>> @@ -77,6 +75,11 @@ void dummy_physical_memory_cleanup(void)
>>   	free(memory_block.base);
>>   }
>>   
>> +void *get_memory_block_base(void)
>> +{
>> +	return 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 d396e5423a8e..d56af621c543 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
>>   
>>   /**
>>    * ASSERT_EQ():
>> @@ -73,6 +75,7 @@ void reset_memblock_attributes(void);
>>   void setup_memblock(void);
>>   void dummy_physical_memory_init(void);
>>   void dummy_physical_memory_cleanup(void);
>> +void *get_memory_block_base(void);
> 
> Let's make it
> 
> phys_addr_t dummy_physical_memory_base(void);
> 

Got it.

>>   void parse_args(int argc, char **argv);
>>   
>>   void test_fail(void);
>> -- 
>> 2.34.1
>>
>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] memblock test: Add test to memblock_reserve() 129th region
  2022-09-01  8:08   ` Mike Rapoport
@ 2022-09-01  8:43     ` Huang, Shaoqin
  0 siblings, 0 replies; 9+ messages in thread
From: Huang, Shaoqin @ 2022-09-01  8:43 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Karolina Drobnik, Rebecca Mckeever, David Hildenbrand, linux-mm,
	linux-kernel



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
>>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] memblock test: Add test to memblock_add() 129th region
  2022-08-22  2:33 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
@ 2022-08-22  2:33 ` shaoqin.huang
  0 siblings, 0 replies; 9+ messages in thread
From: shaoqin.huang @ 2022-08-22  2:33 UTC (permalink / raw)
  To: rppt
  Cc: Shaoqin Huang, Karolina Drobnik, Rebecca Mckeever,
	David Hildenbrand, linux-mm, linux-kernel

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 a valid memory region, and
fake the other memory region, so 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 | 82 ++++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |  7 +-
 tools/testing/memblock/tests/common.h    |  3 +
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index 66f46f261e66..c8e201156cdc 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -326,6 +326,87 @@ static int memblock_add_twice_check(void)
 	return 0;
 }
 
+/*
+ * A test that tries 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,
+	};
+	phys_addr_t memory_base = SZ_128K;
+
+	PREFIX_PUSH();
+
+	reset_memblock_regions();
+	memblock_allow_resize();
+
+	/*
+	 * Add one valid memory region, this will be choosed to be the memory
+	 * that new memory.regions occupied.
+	 */
+	dummy_physical_memory_init();
+	memblock_add((phys_addr_t)get_memory_block_base(), MEM_SIZE);
+
+	ASSERT_EQ(memblock.memory.cnt, 1);
+	ASSERT_EQ(memblock.memory.total_size, MEM_SIZE);
+
+	for (i = 1; i < INIT_MEMBLOCK_REGIONS; i++) {
+		/* Add some fakes memory region to fulfill the memblock. */
+		memblock_add(memory_base, MEM_SIZE);
+
+		ASSERT_EQ(memblock.memory.cnt, i + 1);
+		ASSERT_EQ(memblock.memory.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.memory.regions;
+
+	/* This adds the 129 memory_region, and makes it double array. */
+	memblock_add((phys_addr_t)memory_base, 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)));
+
+	/* The base is very small, so it should be insert to the first region. */
+	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);
+
+	dummy_physical_memory_cleanup();
+
+	/*
+	 * 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();
@@ -339,6 +420,7 @@ static int memblock_add_checks(void)
 	memblock_add_overlap_bottom_check();
 	memblock_add_within_check();
 	memblock_add_twice_check();
+	memblock_add_many_check();
 
 	prefix_pop();
 
diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
index e43b2676af81..89684ec00aa2 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					": "
 
@@ -75,6 +73,11 @@ void dummy_physical_memory_cleanup(void)
 	free(memory_block.base);
 }
 
+void *get_memory_block_base(void)
+{
+	return 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 3e7f23d341d7..98b0a0ec629d 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
 
 /**
  * ASSERT_EQ():
@@ -70,6 +72,7 @@ void reset_memblock_attributes(void);
 void setup_memblock(void);
 void dummy_physical_memory_init(void);
 void dummy_physical_memory_cleanup(void);
+void *get_memory_block_base(void);
 void parse_args(int argc, char **argv);
 
 void test_fail(void);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-09-01  8:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-30  1:49 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
2022-08-30  1:49 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang
2022-09-01  8:02   ` Mike Rapoport
2022-09-01  8:35     ` Huang, Shaoqin
2022-08-30  1:49 ` [PATCH 2/3] memblock test: Add test to memblock_reserve() " shaoqin.huang
2022-09-01  8:08   ` Mike Rapoport
2022-09-01  8:43     ` Huang, Shaoqin
2022-08-30  1:49 ` [PATCH 3/3] memblock test: Update TODO list shaoqin.huang
  -- strict thread matches above, loose matches on Subject: below --
2022-08-22  2:33 [PATCH 0/3] Add tests trying to memblock_add() or memblock_reserve() 129th region shaoqin.huang
2022-08-22  2:33 ` [PATCH 1/3] memblock test: Add test to memblock_add() " shaoqin.huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).