Skip to contentSkip to conversation

memblock tests: update and extend memblock simulator

PATCH Superseded Rebecca Mckeever 3 participants 18 messages Aug 19, 2022 – Aug 26, 2022

Subsystems (2)
Memory management memblock and memory management initialization

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Rebecca Mckeever

author
#0 lore raw

v2 cover memblock tests: update and extend memblock simulator

These patches update existing tests in memblock simulator, add additional tests for memblock functions that are already being tested, and add test coverage for additional memblock functions.

Updated tests for:
- memblock_alloc()
- memblock_alloc_try_nid()
- memblock_alloc_from()

The updates to memblock_alloc() tests include the addition of an assert that checks whether the entire chunk of allocated memory is cleared. For memblock_alloc_try_nid() and memblock_alloc_from(), the assert that checks whether the allocated memory is cleared now checks the entire chunk of allocated memory instead of just the first byte. To make this more robust, setup_memblock() and dummy_physical_memory_init() fill the entire MEM_SIZE simulated physical memory with nonzero values by calling fill_memblock(). setup_memblock() is called at the beginning of most tests for memblock_alloc() functions.

Additional tests for:
- memblock_add()
- memblock_reserve()
- memblock_remove()
- memblock_free()
- memblock_alloc()

Introducing test coverage for:
- memblock_alloc_raw()
- memblock_alloc_try_nid_raw()
- memblock_set_bottom_up()
- memblock_bottom_up()
- memblock_trim_memory()

The tests for the memblock_alloc_*raw() functions test both top-down and bottom-up allocation directions. To add coverage for memblock_alloc_raw(), the alloc_api was updated so that it runs through all the existing tests
twice: once for memblock_alloc() and once for memblock_alloc_raw(). When the tests run memblock_alloc_raw(), they test that the entire memory region is nonzero instead of testing that it is zero.

Similarly, the alloc_nid_api was updated to run through its tests twice:
once for memblock_alloc_try_nid() and once for memblock_alloc_try_nid_raw(). When the tests run memblock_alloc_try_nid_raw(), they test that the entire memory region is nonzero instead of testing that it is zero.

The patch set also adds labels to verbose output for generic memblock_alloc*() tests that indicate which allocation direction is set. The function names of those tests do not include this information.

---
Changelog

v1 -> v2
Updates based on feedback from Shaoqin Huang:
PATCH 1:
- tests/alloc_api.c:
- Remove fill_memblock() from alloc_no_memory_generic_check().
- tests/common.c, tests/common.h:
- Change fill_memblock() to file static.
PATCH 3:
- Shaoqin Huang and I discussed using run_top_down() and run_bottom_up() even for functions with top_down and bottom_up in the name to maintain a consistent output style. However, this would make the output more redundant, so no changes were made.
PATCH 4:
- tests/basic_api.c:
- Rename instances of r1_size and r2_size to
new_r1_size and new_r2_size.
PATCH 6:
- tests/alloc_api.c, tests/alloc_nid_api.c, tests/common.h:
- Change verify_mem_content() to a common function defined in common.h.
PATCH 8:
- tests/basic_api.c:
- Rename instances of r2_base and r2_size to
new_r2_base and new_r2_size.
---

Rebecca Mckeever (8):
memblock tests: update tests to check if memblock_alloc zeroed memory memblock tests: update zeroed memory check for memblock_alloc_* tests memblock tests: add labels to verbose output for generic alloc tests memblock tests: add additional tests for basic api and memblock_alloc memblock tests: update alloc_api to test memblock_alloc_raw memblock tests: update alloc_nid_api to test
memblock_alloc_try_nid_raw
memblock tests: add tests for memblock_*bottom_up functions memblock tests: add tests for memblock_trim_memory

 tools/testing/memblock/tests/alloc_api.c      | 175 +++-
 .../memblock/tests/alloc_helpers_api.c        |  20 +-
 tools/testing/memblock/tests/alloc_nid_api.c  | 260 +++---
 tools/testing/memblock/tests/basic_api.c      | 767 ++++++++++++++++++
 tools/testing/memblock/tests/common.c         |   7 +
 tools/testing/memblock/tests/common.h         |  53 ++
 6 files changed, 1095 insertions(+), 187 deletions(-)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v2 1/8 memblock tests: update tests to check if memblock_alloc zeroed memory

Add an assert in memblock_alloc() tests where allocation is expected to occur. The assert checks whether the entire chunk of allocated memory is cleared.

The current memblock_alloc() tests do not check whether the allocated memory was zeroed. memblock_alloc() should zero the allocated memory since it is a wrapper for memblock_alloc_try_nid().

Reviewed-by Shaoqin Huang <[email protected]>
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 23 +++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |  7 +++++++
 tools/testing/memblock/tests/common.h    | 12 ++++++++++++
 3 files changed, 42 insertions(+)
tools/testing/memblock/tests/alloc_api.c +23 -0
index a14f38eb8a89..aefb67557de9 100644
@@ -22,6 +22,8 @@ static int alloc_top_down_simple_check(void)
 	allocated_ptr = memblock_alloc(size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, expected_start);
 

@@ -80,6 +82,8 @@ static int alloc_top_down_disjoint_check(void)
 	allocated_ptr = memblock_alloc(r2_size, alignment);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, r1.base);
 

@@ -125,6 +129,8 @@ static int alloc_top_down_before_check(void)
 	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - total_size);
 

@@ -173,6 +179,8 @@ static int alloc_top_down_after_check(void)
 	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base - r2_size);
 

@@ -223,6 +231,8 @@ static int alloc_top_down_second_fit_check(void)
 	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+
 	ASSERT_EQ(rgn->size, r2.size + r3_size);
 	ASSERT_EQ(rgn->base, r2.base - r3_size);
 

@@ -277,6 +287,8 @@ static int alloc_in_between_generic_check(void)
 	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base - r2.size - r3_size);
 

@@ -418,6 +430,8 @@ static int alloc_limited_space_generic_check(void)
 	allocated_ptr = memblock_alloc(available_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, available_size);
+
 	ASSERT_EQ(rgn->size, MEM_SIZE);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
 

@@ -472,6 +486,8 @@ static int alloc_bottom_up_simple_check(void)
 	allocated_ptr = memblock_alloc(SZ_2, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, SZ_2);
+
 	ASSERT_EQ(rgn->size, SZ_2);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
 

@@ -528,6 +544,7 @@ static int alloc_bottom_up_disjoint_check(void)
 	allocated_ptr = memblock_alloc(r2_size, alignment);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
 
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, r1.base);

@@ -571,6 +588,8 @@ static int alloc_bottom_up_before_check(void)
 	allocated_ptr = memblock_alloc(r1_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r1_size);
+
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
 

@@ -618,6 +637,8 @@ static int alloc_bottom_up_after_check(void)
 	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base);
 

@@ -669,6 +690,8 @@ static int alloc_bottom_up_second_fit_check(void)
 	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+
 	ASSERT_EQ(rgn->size, r2.size + r3_size);
 	ASSERT_EQ(rgn->base, r2.base);
tools/testing/memblock/tests/common.c +7 -0
index 76a8ad818f3a..eec6901081af 100644
@@ -60,16 +60,23 @@ void reset_memblock_attributes(void)
 	memblock.current_limit	= MEMBLOCK_ALLOC_ANYWHERE;
 }
 
+static inline void fill_memblock(void)
+{
+	memset(memory_block.base, 1, MEM_SIZE);
+}
+
 void setup_memblock(void)
 {
 	reset_memblock_regions();
 	memblock_add((phys_addr_t)memory_block.base, MEM_SIZE);
+	fill_memblock();
 }
 
 void dummy_physical_memory_init(void)
 {
 	memory_block.base = malloc(MEM_SIZE);
 	assert(memory_block.base);
+	fill_memblock();
 }
 
 void dummy_physical_memory_cleanup(void)
tools/testing/memblock/tests/common.h +12 -0
index d396e5423a8e..29756e652e3e 100644
@@ -51,6 +51,18 @@
 	assert((_expected) < (_seen)); \
 } while (0)
 
+/**
+ * ASSERT_MEM_EQ():
+ * Check that the first @_size bytes of @_seen are all equal to @_expected.
+ * If false, print failed test message (if running with --verbose) and then
+ * assert.
+ */
+#define ASSERT_MEM_EQ(_seen, _expected, _size) do { \
+	for (int _i = 0; _i < (_size); _i++) { \
+		ASSERT_EQ((_seen)[_i], (_expected)); \
+	} \
+} while (0)
+
 #define PREFIX_PUSH() prefix_push(__func__)
 
 /*
Signature · 1 line
2.25.1
Reviewed-by on v2 1/8
#2 lore raw

Re: [PATCH v2 1/8] memblock tests: update tests to check if memblock_alloc zeroed memory

Quoted 10 lines · Rebecca Mckeever

Add an assert in memblock_alloc() tests where allocation is expected to occur. The assert checks whether the entire chunk of allocated memory is cleared.

The current memblock_alloc() tests do not check whether the allocated memory was zeroed. memblock_alloc() should zero the allocated memory since it is a wrapper for memblock_alloc_try_nid().

Reviewed-by Shaoqin Huang <[email protected]>
Signed-off-by Rebecca Mckeever <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#3 lore raw

Re: [PATCH v2 1/8] memblock tests: update tests to check if memblock_alloc zeroed memory

In reply to Rebecca Mckeever
Quoted 21 lines · Rebecca Mckeever

Add an assert in memblock_alloc() tests where allocation is expected to occur. The assert checks whether the entire chunk of allocated memory is cleared.

The current memblock_alloc() tests do not check whether the allocated memory was zeroed. memblock_alloc() should zero the allocated memory since it is a wrapper for memblock_alloc_try_nid().

Reviewed-by Shaoqin Huang <[email protected]>
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 23 +++++++++++++++++++++++
 tools/testing/memblock/tests/common.c    |  7 +++++++
 tools/testing/memblock/tests/common.h    | 12 ++++++++++++
 3 files changed, 42 insertions(+)
tools/testing/memblock/tests/alloc_api.c +0 -0
index a14f38eb8a89..aefb67557de9 100644

tools/testing/memblock/tests/alloc_api.c static int alloc_top_down_simple_check(void) · line 26

Quoted 5 lines
tools/testing/memblock/tests/alloc_api.c +2 -0
@@ -22,6 +22,8 @@ static int alloc_top_down_simple_check(void)
 	allocated_ptr = memblock_alloc(size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+

Can we please hide the casting inside ASSERT_MEM_EQ()?

Like if ASSERT_MEM_EQ() were a function its declaration would be

bool ASSERT_MEM_EQ(void *mem, char val, size_t size);

Quoted 3 lines
tools/testing/memblock/tests/alloc_api.c +0 -0
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, expected_start);
Signature · 2 lines
Sincerely yours,
Mike.
author
#4 lore raw

PATCH v2 2/8 memblock tests: update zeroed memory check for memblock_alloc_* tests

In reply to Rebecca Mckeever

Update the assert in memblock_alloc_try_nid() and memblock_alloc_from() tests that checks whether the memory is cleared so that it checks the entire chunk of allocated memory instead of just the first byte.

Reviewed-by David Hildenbrand <[email protected]>
Reviewed-by Shaoqin Huang <[email protected]>
Signed-off-by Rebecca Mckeever <[email protected]>
---
 .../memblock/tests/alloc_helpers_api.c        |  8 +--
 tools/testing/memblock/tests/alloc_nid_api.c  | 72 +++++--------------
 2 files changed, 20 insertions(+), 60 deletions(-)
tools/testing/memblock/tests/alloc_helpers_api.c +2 -6
index 1069b4bdd5fd..796527cf3bd2 100644
@@ -19,7 +19,6 @@ static int alloc_from_simple_generic_check(void)
 {
 	struct memblock_region *rgn = &memblock.reserved.regions[0];
 	void *allocated_ptr = NULL;
-	char *b;
 
 	PREFIX_PUSH();
 

@@ -31,10 +30,9 @@ static int alloc_from_simple_generic_check(void)
 	min_addr = memblock_end_of_DRAM() - SMP_CACHE_BYTES;
 
 	allocated_ptr = memblock_alloc_from(size, SMP_CACHE_BYTES, min_addr);
-	b = (char *)allocated_ptr;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_EQ(*b, 0);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, min_addr);

@@ -66,7 +64,6 @@ static int alloc_from_misaligned_generic_check(void)
 {
 	struct memblock_region *rgn = &memblock.reserved.regions[0];
 	void *allocated_ptr = NULL;
-	char *b;
 
 	PREFIX_PUSH();
 

@@ -79,10 +76,9 @@ static int alloc_from_misaligned_generic_check(void)
 	min_addr = memblock_end_of_DRAM() - (SMP_CACHE_BYTES * 2 - 1);
 
 	allocated_ptr = memblock_alloc_from(size, SMP_CACHE_BYTES, min_addr);
-	b = (char *)allocated_ptr;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_EQ(*b, 0);
+	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - SMP_CACHE_BYTES);
tools/testing/memblock/tests/alloc_nid_api.c +18 -54

329 lines not on this page · raw message

Signature · 1 line
2.25.1
author
#5 lore raw

PATCH v2 3/8 memblock tests: add labels to verbose output for generic alloc tests

In reply to Rebecca Mckeever

Generic tests for memblock_alloc*() functions do not use separate functions for testing top-down and bottom-up allocation directions. Therefore, the function name that is displayed in the verbose testing output does not include the allocation direction.

Add an additional prefix when running generic tests for memblock_alloc*() functions that indicates which allocation direction is set. The prefix will be displayed when the tests are run in verbose mode.

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c      | 36 +++++++------------
 .../memblock/tests/alloc_helpers_api.c        | 12 +++----
 tools/testing/memblock/tests/alloc_nid_api.c  | 36 +++++++------------
 tools/testing/memblock/tests/common.h         | 16 +++++++++
 4 files changed, 44 insertions(+), 56 deletions(-)
tools/testing/memblock/tests/alloc_api.c +12 -24
index aefb67557de9..6a9c8e788cac 100644
@@ -751,10 +751,8 @@ static int alloc_after_check(void)
 static int alloc_in_between_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_in_between_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_in_between_generic_check();
+	run_top_down(alloc_in_between_generic_check);
+	run_bottom_up(alloc_in_between_generic_check);
 
 	return 0;
 }

@@ -773,10 +771,8 @@ static int alloc_second_fit_check(void)
 static int alloc_small_gaps_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_small_gaps_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_small_gaps_generic_check();
+	run_top_down(alloc_small_gaps_generic_check);
+	run_bottom_up(alloc_small_gaps_generic_check);
 
 	return 0;
 }

@@ -784,10 +780,8 @@ static int alloc_small_gaps_check(void)
 static int alloc_all_reserved_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_all_reserved_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_all_reserved_generic_check();
+	run_top_down(alloc_all_reserved_generic_check);
+	run_bottom_up(alloc_all_reserved_generic_check);
 
 	return 0;
 }

@@ -795,10 +789,8 @@ static int alloc_all_reserved_check(void)
 static int alloc_no_space_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_no_space_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_no_space_generic_check();
+	run_top_down(alloc_no_space_generic_check);
+	run_bottom_up(alloc_no_space_generic_check);
 
 	return 0;
 }

@@ -806,10 +798,8 @@ static int alloc_no_space_check(void)
 static int alloc_limited_space_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_limited_space_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_limited_space_generic_check();
+	run_top_down(alloc_limited_space_generic_check);
+	run_bottom_up(alloc_limited_space_generic_check);
 
 	return 0;
 }

@@ -817,10 +807,8 @@ static int alloc_limited_space_check(void)
 static int alloc_no_memory_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_no_memory_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_no_memory_generic_check();
+	run_top_down(alloc_no_memory_generic_check);
+	run_bottom_up(alloc_no_memory_generic_check);
 
 	return 0;
 }
tools/testing/memblock/tests/alloc_helpers_api.c +4 -8
index 796527cf3bd2..1ccf02639ad6 100644
@@ -357,10 +357,8 @@ static int alloc_from_bottom_up_min_addr_cap_check(void)
 static int alloc_from_simple_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_from_simple_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_from_simple_generic_check();
+	run_top_down(alloc_from_simple_generic_check);
+	run_bottom_up(alloc_from_simple_generic_check);
 
 	return 0;
 }

@@ -368,10 +366,8 @@ static int alloc_from_simple_check(void)
 static int alloc_from_misaligned_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_from_misaligned_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_from_misaligned_generic_check();
+	run_top_down(alloc_from_misaligned_generic_check);
+	run_bottom_up(alloc_from_misaligned_generic_check);
 
 	return 0;
 }
tools/testing/memblock/tests/alloc_nid_api.c +12 -24
index 71b7beb35526..82fa8ea36320 100644
@@ -1142,10 +1142,8 @@ static int alloc_try_nid_cap_min_check(void)
 static int alloc_try_nid_min_reserved_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_min_reserved_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_min_reserved_generic_check();
+	run_top_down(alloc_try_nid_min_reserved_generic_check);
+	run_bottom_up(alloc_try_nid_min_reserved_generic_check);
 
 	return 0;
 }

@@ -1153,10 +1151,8 @@ static int alloc_try_nid_min_reserved_check(void)
 static int alloc_try_nid_max_reserved_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_max_reserved_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_max_reserved_generic_check();
+	run_top_down(alloc_try_nid_max_reserved_generic_check);
+	run_bottom_up(alloc_try_nid_max_reserved_generic_check);
 
 	return 0;
 }

@@ -1164,10 +1160,8 @@ static int alloc_try_nid_max_reserved_check(void)
 static int alloc_try_nid_exact_address_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_exact_address_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_exact_address_generic_check();
+	run_top_down(alloc_try_nid_exact_address_generic_check);
+	run_bottom_up(alloc_try_nid_exact_address_generic_check);
 
 	return 0;
 }

@@ -1175,10 +1169,8 @@ static int alloc_try_nid_exact_address_check(void)
 static int alloc_try_nid_reserved_full_merge_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_reserved_full_merge_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_reserved_full_merge_generic_check();
+	run_top_down(alloc_try_nid_reserved_full_merge_generic_check);
+	run_bottom_up(alloc_try_nid_reserved_full_merge_generic_check);
 
 	return 0;
 }

@@ -1186,10 +1178,8 @@ static int alloc_try_nid_reserved_full_merge_check(void)
 static int alloc_try_nid_reserved_all_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_reserved_all_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_reserved_all_generic_check();
+	run_top_down(alloc_try_nid_reserved_all_generic_check);
+	run_bottom_up(alloc_try_nid_reserved_all_generic_check);
 
 	return 0;
 }

@@ -1197,10 +1187,8 @@ static int alloc_try_nid_reserved_all_check(void)
 static int alloc_try_nid_low_max_check(void)
 {
 	test_print("\tRunning %s...\n", __func__);
-	memblock_set_bottom_up(false);
-	alloc_try_nid_low_max_generic_check();
-	memblock_set_bottom_up(true);
-	alloc_try_nid_low_max_generic_check();
+	run_top_down(alloc_try_nid_low_max_generic_check);
+	run_bottom_up(alloc_try_nid_low_max_generic_check);
 
 	return 0;
 }
tools/testing/memblock/tests/common.h +16 -0
index 29756e652e3e..58f84bf2c9ae 100644
@@ -100,4 +100,20 @@ static inline void test_pass_pop(void)
 	prefix_pop();
 }
 
+static inline void run_top_down(int (*func)())
+{
+	memblock_set_bottom_up(false);
+	prefix_push("top-down");
+	func();
+	prefix_pop();
+}
+
+static inline void run_bottom_up(int (*func)())
+{
+	memblock_set_bottom_up(true);
+	prefix_push("bottom-up");
+	func();
+	prefix_pop();
+}
+
 #endif
Signature · 1 line
2.25.1
Reviewed-by on v2 3/8
#6 lore raw

Re: [PATCH v2 3/8] memblock tests: add labels to verbose output for generic alloc tests

Quoted 10 lines · Rebecca Mckeever

Generic tests for memblock_alloc*() functions do not use separate functions for testing top-down and bottom-up allocation directions. Therefore, the function name that is displayed in the verbose testing output does not include the allocation direction.

Add an additional prefix when running generic tests for memblock_alloc*() functions that indicates which allocation direction is set. The prefix will be displayed when the tests are run in verbose mode.

Signed-off-by Rebecca Mckeever <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks,

David / dhildenb
author
#7 lore raw

PATCH v2 4/8 memblock tests: add additional tests for basic api and memblock_alloc

In reply to Rebecca Mckeever

Add tests for memblock_add(), memblock_reserve(), memblock_remove(), memblock_free(), and memblock_alloc() for the following test scenarios.

memblock_add() and memblock_reserve():
- add/reserve a memory block in the gap between two existing memory blocks, and check that the blocks are merged into one region
- try to add/reserve memblock regions that extend past PHYS_ADDR_MAX

memblock_remove() and memblock_free():
- remove/free a region when it is the only available region
+ These tests ensure that the first region is overwritten with a "dummy" region when the last remaining region of that type is removed or freed.
- remove/free() a region that overlaps with two existing regions of the relevant type
- try to remove/free memblock regions that extend past PHYS_ADDR_MAX

memblock_alloc():
- try to allocate a region that is larger than the total size of available memory (memblock.memory)

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c |  44 ++
 tools/testing/memblock/tests/basic_api.c | 499 +++++++++++++++++++++++
 2 files changed, 543 insertions(+)
tools/testing/memblock/tests/alloc_api.c +44 -0
index 6a9c8e788cac..65bff77dd55b 100644
@@ -469,6 +469,40 @@ static int alloc_no_memory_generic_check(void)
 	return 0;
 }
 
+/*
+ * A test that tries to allocate a region that is larger than the total size of
+ * available memory (memblock.memory):
+ *
+ *  +-----------------------------------+
+ *  |                 new               |
+ *  +-----------------------------------+
+ *  |                                 |
+ *  |                                 |
+ *  +---------------------------------+
+ *
+ * Expect no allocation to happen.
+ */
+static int alloc_too_large_generic_check(void)
+{
+	struct memblock_region *rgn = &memblock.reserved.regions[0];
+	void *allocated_ptr = NULL;
+
+	PREFIX_PUSH();
+
+	setup_memblock();
+
+	allocated_ptr = memblock_alloc(MEM_SIZE + SZ_2, SMP_CACHE_BYTES);
+
+	ASSERT_EQ(allocated_ptr, NULL);
+	ASSERT_EQ(rgn->size, 0);
+	ASSERT_EQ(rgn->base, 0);
+	ASSERT_EQ(memblock.reserved.total_size, 0);
+
+	test_pass_pop();
+
+	return 0;
+}
+
 /*
  * A simple test that tries to allocate a small memory region.
  * Expect to allocate an aligned region at the beginning of the available

@@ -813,6 +847,15 @@ static int alloc_no_memory_check(void)
 	return 0;
 }
 
+static int alloc_too_large_check(void)
+{
+	test_print("\tRunning %s...\n", __func__);
+	run_top_down(alloc_too_large_generic_check);
+	run_bottom_up(alloc_too_large_generic_check);
+
+	return 0;
+}
+
 int memblock_alloc_checks(void)
 {
 	const char *func_testing = "memblock_alloc";

@@ -835,6 +878,7 @@ int memblock_alloc_checks(void)
 	alloc_no_space_check();
 	alloc_limited_space_check();
 	alloc_no_memory_check();
+	alloc_too_large_check();
 
 	dummy_physical_memory_cleanup();
tools/testing/memblock/tests/basic_api.c +499 -0

547 lines not on this page · raw message

Signature · 1 line
2.25.1
Reviewed-by on v2 4/8
#8 lore raw

Re: [PATCH v2 4/8] memblock tests: add additional tests for basic api and memblock_alloc

Quoted 17 lines · Rebecca Mckeever

Add tests for memblock_add(), memblock_reserve(), memblock_remove(), memblock_free(), and memblock_alloc() for the following test scenarios.

memblock_add() and memblock_reserve():
- add/reserve a memory block in the gap between two existing memory blocks, and check that the blocks are merged into one region
- try to add/reserve memblock regions that extend past PHYS_ADDR_MAX

memblock_remove() and memblock_free():
- remove/free a region when it is the only available region
+ These tests ensure that the first region is overwritten with a "dummy" region when the last remaining region of that type is removed or freed.
- remove/free() a region that overlaps with two existing regions of the relevant type
- try to remove/free memblock regions that extend past PHYS_ADDR_MAX

Quoted 5 lines

memblock_alloc():
- try to allocate a region that is larger than the total size of available memory (memblock.memory)

Signed-off-by Rebecca Mckeever <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Signature · 3 lines
Thanks,

David / dhildenb
author
#9 lore raw

PATCH v2 5/8 memblock tests: update alloc_api to test memblock_alloc_raw

In reply to Rebecca Mckeever

Update memblock_alloc() tests so that they test either memblock_alloc() or memblock_alloc_raw() depending on the value of alloc_test_flags. Run through all the existing tests in memblock_alloc_api twice: once for memblock_alloc() and once for memblock_alloc_raw().

When the tests run memblock_alloc(), they test that the entire memory region is zero. When the tests run memblock_alloc_raw(), they test that the entire memory region is nonzero.

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
 tools/testing/memblock/tests/common.h    | 25 ++++++
 2 files changed, 90 insertions(+), 33 deletions(-)
tools/testing/memblock/tests/alloc_api.c +65 -33
index 65bff77dd55b..cf67687ae044 100644
@@ -1,6 +1,29 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc",
+	"memblock_alloc_raw"
+};
+
+static int alloc_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];
+}
+
+static inline void *run_memblock_alloc(phys_addr_t size, phys_addr_t align)
+{
+	if (alloc_test_flags & TEST_RAW)
+		return memblock_alloc_raw(size, align);
+	else
+		return memblock_alloc(size, align);
+}
+
 /*
  * A simple test that tries to allocate a small memory region.
  * Expect to allocate an aligned region near the end of the available memory.

@@ -19,10 +42,10 @@ static int alloc_top_down_simple_check(void)
 
 	expected_start = memblock_end_of_DRAM() - SMP_CACHE_BYTES;
 
-	allocated_ptr = memblock_alloc(size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, expected_start);

@@ -79,10 +102,10 @@ static int alloc_top_down_disjoint_check(void)
 
 	memblock_reserve(r1.base, r1.size);
 
-	allocated_ptr = memblock_alloc(r2_size, alignment);
+	allocated_ptr = run_memblock_alloc(r2_size, alignment);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, r1.base);

@@ -126,10 +149,10 @@ static int alloc_top_down_before_check(void)
 
 	memblock_reserve(memblock_end_of_DRAM() - total_size, r1_size);
 
-	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - total_size);

@@ -176,10 +199,10 @@ static int alloc_top_down_after_check(void)
 
 	memblock_reserve(r1.base, r1.size);
 
-	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base - r2_size);

@@ -228,10 +251,10 @@ static int alloc_top_down_second_fit_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, r2.size + r3_size);
 	ASSERT_EQ(rgn->base, r2.base - r3_size);

@@ -284,10 +307,10 @@ static int alloc_in_between_generic_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base - r2.size - r3_size);

@@ -332,7 +355,7 @@ static int alloc_small_gaps_generic_check(void)
 		region_end += gap_size + region_size;
 	}
 
-	allocated_ptr = memblock_alloc(region_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(region_size, SMP_CACHE_BYTES);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 

@@ -356,7 +379,7 @@ static int alloc_all_reserved_generic_check(void)
 	/* Simulate full memory */
 	memblock_reserve(memblock_start_of_DRAM(), MEM_SIZE);
 
-	allocated_ptr = memblock_alloc(SZ_256, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(SZ_256, SMP_CACHE_BYTES);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 

@@ -392,7 +415,7 @@ static int alloc_no_space_generic_check(void)
 	/* Simulate almost-full memory */
 	memblock_reserve(memblock_start_of_DRAM(), reserved_size);
 
-	allocated_ptr = memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 

@@ -427,10 +450,10 @@ static int alloc_limited_space_generic_check(void)
 	/* Simulate almost-full memory */
 	memblock_reserve(memblock_start_of_DRAM(), reserved_size);
 
-	allocated_ptr = memblock_alloc(available_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(available_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, available_size);
+	verify_mem_content(allocated_ptr, available_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, MEM_SIZE);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());

@@ -457,7 +480,7 @@ static int alloc_no_memory_generic_check(void)
 
 	reset_memblock_regions();
 
-	allocated_ptr = memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 	ASSERT_EQ(rgn->size, 0);

@@ -491,7 +514,7 @@ static int alloc_too_large_generic_check(void)
 
 	setup_memblock();
 
-	allocated_ptr = memblock_alloc(MEM_SIZE + SZ_2, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(MEM_SIZE + SZ_2, SMP_CACHE_BYTES);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 	ASSERT_EQ(rgn->size, 0);

@@ -517,10 +540,10 @@ static int alloc_bottom_up_simple_check(void)
 
 	setup_memblock();
 
-	allocated_ptr = memblock_alloc(SZ_2, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(SZ_2, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, SZ_2);
+	verify_mem_content(allocated_ptr, SZ_2, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, SZ_2);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());

@@ -575,10 +598,10 @@ static int alloc_bottom_up_disjoint_check(void)
 
 	memblock_reserve(r1.base, r1.size);
 
-	allocated_ptr = memblock_alloc(r2_size, alignment);
+	allocated_ptr = run_memblock_alloc(r2_size, alignment);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, r1.base);

@@ -619,10 +642,10 @@ static int alloc_bottom_up_before_check(void)
 
 	memblock_reserve(memblock_start_of_DRAM() + r1_size, r2_size);
 
-	allocated_ptr = memblock_alloc(r1_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r1_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r1_size);
+	verify_mem_content(allocated_ptr, r1_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());

@@ -668,10 +691,10 @@ static int alloc_bottom_up_after_check(void)
 
 	memblock_reserve(r1.base, r1.size);
 
-	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r2_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r1.base);

@@ -721,10 +744,10 @@ static int alloc_bottom_up_second_fit_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
+	allocated_ptr = run_memblock_alloc(r3_size, SMP_CACHE_BYTES);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_test_flags);
 
 	ASSERT_EQ(rgn->size, r2.size + r3_size);
 	ASSERT_EQ(rgn->base, r2.base);

@@ -856,13 +879,14 @@ static int alloc_too_large_check(void)
 	return 0;
 }
 
-int memblock_alloc_checks(void)
+static int memblock_alloc_checks_internal(int flags)
 {
-	const char *func_testing = "memblock_alloc";
+	const char *func = get_func_testing(flags);
 
+	alloc_test_flags = flags;
 	prefix_reset();
-	prefix_push(func_testing);
-	test_print("Running %s tests...\n", func_testing);
+	prefix_push(func);
+	test_print("Running %s tests...\n", func);
 
 	reset_memblock_attributes();
 	dummy_physical_memory_init();

@@ -886,3 +910,11 @@ int memblock_alloc_checks(void)
 
 	return 0;
 }
+
+int memblock_alloc_checks(void)
+{
+	memblock_alloc_checks_internal(TEST_ZEROED);
+	memblock_alloc_checks_internal(TEST_RAW);
+
+	return 0;
+}
tools/testing/memblock/tests/common.h +25 -0
index 58f84bf2c9ae..4fd3534ff955 100644
@@ -12,6 +12,11 @@
 
 #define MEM_SIZE SZ_16K
 
+enum test_flags {
+	TEST_ZEROED = 0x0,
+	TEST_RAW = 0x1
+};
+
 /**
  * ASSERT_EQ():
  * Check the condition

@@ -63,6 +68,18 @@
 	} \
 } while (0)
 
+/**
+ * ASSERT_MEM_NE():
+ * Check that none of the first @_size bytes of @_seen are equal to @_expected.
+ * If false, print failed test message (if running with --verbose) and then
+ * assert.
+ */
+#define ASSERT_MEM_NE(_seen, _expected, _size) do { \
+	for (int _i = 0; _i < (_size); _i++) { \
+		ASSERT_NE((_seen)[_i], (_expected)); \
+	} \
+} while (0)
+
 #define PREFIX_PUSH() prefix_push(__func__)
 
 /*

@@ -116,4 +133,12 @@ static inline void run_bottom_up(int (*func)())
 	prefix_pop();
 }
 
+static inline void verify_mem_content(void *mem, int size, int flags)
+{
+	if (flags & TEST_RAW)
+		ASSERT_MEM_NE((char *)mem, 0, size);
+	else
+		ASSERT_MEM_EQ((char *)mem, 0, size);
+}
+
 #endif
Signature · 1 line
2.25.1
#10 lore raw

Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw

Quoted 8 lines · Rebecca Mckeever

Update memblock_alloc() tests so that they test either memblock_alloc() or memblock_alloc_raw() depending on the value of alloc_test_flags. Run through all the existing tests in memblock_alloc_api twice: once for memblock_alloc() and once for memblock_alloc_raw().

When the tests run memblock_alloc(), they test that the entire memory region is zero. When the tests run memblock_alloc_raw(), they test that the entire memory region is nonzero.

Could add a comment stating that we initialize the content to nonzero in that case, and expect it to remain unchanged (== not zeroed).

Quoted 18 lines
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
 tools/testing/memblock/tests/common.h    | 25 ++++++
 2 files changed, 90 insertions(+), 33 deletions(-)
tools/testing/memblock/tests/alloc_api.c +3 -0
index 65bff77dd55b..cf67687ae044 100644
@@ -1,6 +1,29 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc",
+	"memblock_alloc_raw"

tools/testing/memblock/tests/alloc_api.c line 16

Quoted 10 lines
tools/testing/memblock/tests/alloc_api.c +10 -0
+};
+
+static int alloc_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];

No need for the else, you can return directly.

Can we avoid the func_testing array?

Persoally, I consider the "get_func_testing()" name a bit confusing.

get_memblock_alloc_name() ?

tools/testing/memblock/tests/common.h line 18

Quoted 12 lines
tools/testing/memblock/tests/common.h +4 -0
index 58f84bf2c9ae..4fd3534ff955 100644
@@ -12,6 +12,11 @@
 
 #define MEM_SIZE SZ_16K
 
+enum test_flags {
+	TEST_ZEROED = 0x0,
+	TEST_RAW = 0x1
+};

I'd have called this

enum test_flags {
	/* No special request. */
	TEST_F_NONE = 0x0,
	/* Perform raw allocations (no zeroing of memory).
	TEST_F_RAW = 0x1,
};

Further, I'd just have use #define for the flags.

Quoted 24 lines
tools/testing/memblock/tests/common.h +13 -0
+
 /**
  * ASSERT_EQ():
  * Check the condition

@@ -63,6 +68,18 @@
 	} \
 } while (0)
 
+/**
+ * ASSERT_MEM_NE():
+ * Check that none of the first @_size bytes of @_seen are equal to @_expected.
+ * If false, print failed test message (if running with --verbose) and then
+ * assert.
+ */
+#define ASSERT_MEM_NE(_seen, _expected, _size) do { \
+	for (int _i = 0; _i < (_size); _i++) { \
+		ASSERT_NE((_seen)[_i], (_expected)); \
+	} \
+} while (0)
+
 #define PREFIX_PUSH() prefix_push(__func__)
 
 /*

tools/testing/memblock/tests/common.h static inline void run_bottom_up(int (*func)()) · line 136

Quoted 4 lines
tools/testing/memblock/tests/common.h +1 -0
@@ -116,4 +133,12 @@ static inline void run_bottom_up(int (*func)())
 	prefix_pop();
 }
 
+static inline void verify_mem_content(void *mem, int size, int flags)

nit: why use verify here when the other functions "assert". I'd have called this something like "assert_mem_content()"

Signature · 3 lines
Thanks,

David / dhildenb
author
#11 lore raw

Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw

Quoted 67 lines · David Hildenbrand
Quoted 8 lines · Rebecca Mckeever

Update memblock_alloc() tests so that they test either memblock_alloc() or memblock_alloc_raw() depending on the value of alloc_test_flags. Run through all the existing tests in memblock_alloc_api twice: once for memblock_alloc() and once for memblock_alloc_raw().

When the tests run memblock_alloc(), they test that the entire memory region is zero. When the tests run memblock_alloc_raw(), they test that the entire memory region is nonzero.

Could add a comment stating that we initialize the content to nonzero in that case, and expect it to remain unchanged (== not zeroed).

Quoted 18 lines
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
 tools/testing/memblock/tests/common.h    | 25 ++++++
 2 files changed, 90 insertions(+), 33 deletions(-)
tools/testing/memblock/tests/alloc_api.c +3 -0
index 65bff77dd55b..cf67687ae044 100644
@@ -1,6 +1,29 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc",
+	"memblock_alloc_raw"

tools/testing/memblock/tests/alloc_api.c line 16

Quoted 10 lines
tools/testing/memblock/tests/alloc_api.c +10 -0
+};
+
+static int alloc_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];

No need for the else, you can return directly.

Can we avoid the func_testing array?

Persoally, I consider the "get_func_testing()" name a bit confusing.

get_memblock_alloc_name() ?

tools/testing/memblock/tests/common.h line 18

Quoted 12 lines
tools/testing/memblock/tests/common.h +4 -0
index 58f84bf2c9ae..4fd3534ff955 100644
@@ -12,6 +12,11 @@
 
 #define MEM_SIZE SZ_16K
 
+enum test_flags {
+	TEST_ZEROED = 0x0,
+	TEST_RAW = 0x1
+};

I'd have called this

Quoted 9 lines
enum test_flags {
	/* No special request. */
	TEST_F_NONE = 0x0,
	/* Perform raw allocations (no zeroing of memory).
	TEST_F_RAW = 0x1,
};

Further, I'd just have use #define for the flags.

Do you mean use two #defines instead of the enum? I thought enums were preferred when defining related constants.

Quoted 27 lines
Quoted 27 lines
tools/testing/memblock/tests/common.h +13 -0
+
 /**
  * ASSERT_EQ():
  * Check the condition

@@ -63,6 +68,18 @@
 	} \
 } while (0)
 
+/**
+ * ASSERT_MEM_NE():
+ * Check that none of the first @_size bytes of @_seen are equal to @_expected.
+ * If false, print failed test message (if running with --verbose) and then
+ * assert.
+ */
+#define ASSERT_MEM_NE(_seen, _expected, _size) do { \
+	for (int _i = 0; _i < (_size); _i++) { \
+		ASSERT_NE((_seen)[_i], (_expected)); \
+	} \
+} while (0)
+
 #define PREFIX_PUSH() prefix_push(__func__)
 
 /*

@@ -116,4 +133,12 @@ static inline void run_bottom_up(int (*func)())
 	prefix_pop();
 }
Quoted 12 lines

tools/testing/memblock/tests/common.h static inline void run_bottom_up(int (*func)()) · line 136

Quoted 1 line
tools/testing/memblock/tests/common.h +1 -0
+static inline void verify_mem_content(void *mem, int size, int flags)

nit: why use verify here when the other functions "assert". I'd have called this something like "assert_mem_content()"

Signature · 3 lines
Thanks,

David / dhildenb

Thanks,
Rebecca

#12 lore raw

Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw

Quoted 75 lines · Rebecca Mckeever
Quoted 74 lines · David Hildenbrand
Quoted 8 lines · Rebecca Mckeever

Update memblock_alloc() tests so that they test either memblock_alloc() or memblock_alloc_raw() depending on the value of alloc_test_flags. Run through all the existing tests in memblock_alloc_api twice: once for memblock_alloc() and once for memblock_alloc_raw().

When the tests run memblock_alloc(), they test that the entire memory region is zero. When the tests run memblock_alloc_raw(), they test that the entire memory region is nonzero.

Could add a comment stating that we initialize the content to nonzero in that case, and expect it to remain unchanged (== not zeroed).

Quoted 18 lines
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
 tools/testing/memblock/tests/common.h    | 25 ++++++
 2 files changed, 90 insertions(+), 33 deletions(-)
tools/testing/memblock/tests/alloc_api.c +3 -0
index 65bff77dd55b..cf67687ae044 100644
@@ -1,6 +1,29 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc",
+	"memblock_alloc_raw"

tools/testing/memblock/tests/alloc_api.c line 16

Quoted 10 lines
tools/testing/memblock/tests/alloc_api.c +10 -0
+};
+
+static int alloc_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];

No need for the else, you can return directly.

Can we avoid the func_testing array?

Persoally, I consider the "get_func_testing()" name a bit confusing.

get_memblock_alloc_name() ?

tools/testing/memblock/tests/common.h line 18

Quoted 12 lines
tools/testing/memblock/tests/common.h +4 -0
index 58f84bf2c9ae..4fd3534ff955 100644
@@ -12,6 +12,11 @@
 
 #define MEM_SIZE SZ_16K
 
+enum test_flags {
+	TEST_ZEROED = 0x0,
+	TEST_RAW = 0x1
+};

I'd have called this

enum test_flags {
	/* No special request. */
	TEST_F_NONE = 0x0,
	/* Perform raw allocations (no zeroing of memory).
	TEST_F_RAW = 0x1,
};
Quoted 4 lines
Quoted 2 lines

Further, I'd just have use #define for the flags.

Do you mean use two #defines instead of the enum? I thought enums were preferred when defining related constants.

I guess we have a wild mixture of raw define, enums and __bitwise + defines nowdays.

E.g., take a look at include/linux/rmap.h "typedef int __bitwise rmap_t" and how it's used -- that seems to be the new "best" solution for use in the kernel.

Having that said, feel free to just let it be an enum :)

Signature · 3 lines
Thanks,

David / dhildenb
author
#13 lore raw

PATCH v2 6/8 memblock tests: update alloc_nid_api to test memblock_alloc_try_nid_raw

In reply to Rebecca Mckeever

Update memblock_alloc_try_nid() tests so that they test either memblock_alloc_try_nid() or memblock_alloc_try_nid_raw() depending on the value of alloc_nid_test_flags. Run through all the existing tests in alloc_nid_api twice: once for memblock_alloc_try_nid() and once for memblock_alloc_try_nid_raw().

When the tests run memblock_alloc_try_nid(), they test that the entire memory region is zero. When the tests run memblock_alloc_try_nid_raw(), they test that the entire memory region is nonzero.

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_nid_api.c | 188 ++++++++++++-------
 1 file changed, 119 insertions(+), 69 deletions(-)
tools/testing/memblock/tests/alloc_nid_api.c +119 -69
index 82fa8ea36320..2c1d5035e057 100644
@@ -1,6 +1,34 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_nid_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc_try_nid",
+	"memblock_alloc_try_nid_raw"
+};
+
+static int alloc_nid_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];
+}
+
+static inline void *run_memblock_alloc_try_nid(phys_addr_t size,
+					       phys_addr_t align,
+					       phys_addr_t min_addr,
+					       phys_addr_t max_addr, int nid)
+{
+	if (alloc_nid_test_flags & TEST_RAW)
+		return memblock_alloc_try_nid_raw(size, align, min_addr,
+						  max_addr, nid);
+	else
+		return memblock_alloc_try_nid(size, align, min_addr,
+					      max_addr, nid);
+}
+
 /*
  * A simple test that tries to allocate a memory region within min_addr and
  * max_addr range:

@@ -32,12 +60,13 @@ static int alloc_try_nid_top_down_simple_check(void)
 	min_addr = memblock_start_of_DRAM() + SMP_CACHE_BYTES * 2;
 	max_addr = min_addr + SZ_512;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 	rgn_end = rgn->base + rgn->size;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, max_addr - size);

@@ -86,12 +115,13 @@ static int alloc_try_nid_top_down_end_misaligned_check(void)
 	min_addr = memblock_start_of_DRAM() + SMP_CACHE_BYTES * 2;
 	max_addr = min_addr + SZ_512 + misalign;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 	rgn_end = rgn->base + rgn->size;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, max_addr - size - misalign);

@@ -137,12 +167,13 @@ static int alloc_try_nid_exact_address_generic_check(void)
 	min_addr = memblock_start_of_DRAM() + SMP_CACHE_BYTES;
 	max_addr = min_addr + size;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 	rgn_end = rgn->base + rgn->size;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, min_addr);

@@ -189,11 +220,12 @@ static int alloc_try_nid_top_down_narrow_range_check(void)
 	min_addr = memblock_start_of_DRAM() + SZ_512;
 	max_addr = min_addr + SMP_CACHE_BYTES;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, max_addr - size);

@@ -241,8 +273,9 @@ static int alloc_try_nid_low_max_generic_check(void)
 	min_addr = memblock_start_of_DRAM();
 	max_addr = min_addr + SMP_CACHE_BYTES;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 

@@ -287,11 +320,12 @@ static int alloc_try_nid_min_reserved_generic_check(void)
 
 	memblock_reserve(reserved_base, r1_size);
 
-	allocated_ptr = memblock_alloc_try_nid(r2_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r2_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, reserved_base);

@@ -338,11 +372,12 @@ static int alloc_try_nid_max_reserved_generic_check(void)
 
 	memblock_reserve(max_addr, r1_size);
 
-	allocated_ptr = memblock_alloc_try_nid(r2_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r2_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
+	verify_mem_content(allocated_ptr, r2_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, min_addr);

@@ -402,11 +437,12 @@ static int alloc_try_nid_top_down_reserved_with_space_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn1->size, r1.size + r3_size);
 	ASSERT_EQ(rgn1->base, max_addr - r3_size);

@@ -466,11 +502,12 @@ static int alloc_try_nid_reserved_full_merge_generic_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, total_size);
 	ASSERT_EQ(rgn->base, r2.base);

@@ -531,11 +568,12 @@ static int alloc_try_nid_top_down_reserved_no_space_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, r1.base);

@@ -597,8 +635,9 @@ static int alloc_try_nid_reserved_all_generic_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_EQ(allocated_ptr, NULL);
 

@@ -628,11 +667,12 @@ static int alloc_try_nid_top_down_cap_max_check(void)
 	min_addr = memblock_end_of_DRAM() - SZ_1K;
 	max_addr = memblock_end_of_DRAM() + SZ_256;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - size);

@@ -666,11 +706,12 @@ static int alloc_try_nid_top_down_cap_min_check(void)
 	min_addr = memblock_start_of_DRAM() - SZ_256;
 	max_addr = memblock_end_of_DRAM();
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr, NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - size);

@@ -714,13 +755,13 @@ static int alloc_try_nid_bottom_up_simple_check(void)
 	min_addr = memblock_start_of_DRAM() + SMP_CACHE_BYTES * 2;
 	max_addr = min_addr + SZ_512;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 	rgn_end = rgn->base + rgn->size;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, min_addr);

@@ -769,13 +810,13 @@ static int alloc_try_nid_bottom_up_start_misaligned_check(void)
 	min_addr = memblock_start_of_DRAM() + misalign;
 	max_addr = min_addr + SZ_512;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 	rgn_end = rgn->base + rgn->size;
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, min_addr + (SMP_CACHE_BYTES - misalign));

@@ -822,12 +863,12 @@ static int alloc_try_nid_bottom_up_narrow_range_check(void)
 	min_addr = memblock_start_of_DRAM() + SZ_512;
 	max_addr = min_addr + SMP_CACHE_BYTES;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());

@@ -887,12 +928,12 @@ static int alloc_try_nid_bottom_up_reserved_with_space_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn1->size, r1.size);
 	ASSERT_EQ(rgn1->base, max_addr);

@@ -959,12 +1000,12 @@ static int alloc_try_nid_bottom_up_reserved_no_space_check(void)
 	memblock_reserve(r1.base, r1.size);
 	memblock_reserve(r2.base, r2.size);
 
-	allocated_ptr = memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(r3_size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
+	verify_mem_content(allocated_ptr, r3_size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn3->size, r3_size);
 	ASSERT_EQ(rgn3->base, memblock_start_of_DRAM());

@@ -1004,12 +1045,12 @@ static int alloc_try_nid_bottom_up_cap_max_check(void)
 	min_addr = memblock_start_of_DRAM() + SZ_1K;
 	max_addr = memblock_end_of_DRAM() + SZ_256;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, min_addr);

@@ -1043,12 +1084,12 @@ static int alloc_try_nid_bottom_up_cap_min_check(void)
 	min_addr = memblock_start_of_DRAM();
 	max_addr = memblock_end_of_DRAM() - SZ_256;
 
-	allocated_ptr = memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
-					       min_addr, max_addr,
-					       NUMA_NO_NODE);
+	allocated_ptr = run_memblock_alloc_try_nid(size, SMP_CACHE_BYTES,
+						   min_addr, max_addr,
+						   NUMA_NO_NODE);
 
 	ASSERT_NE(allocated_ptr, NULL);
-	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
+	verify_mem_content(allocated_ptr, size, alloc_nid_test_flags);
 
 	ASSERT_EQ(rgn->size, size);
 	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());

@@ -1193,13 +1234,14 @@ static int alloc_try_nid_low_max_check(void)
 	return 0;
 }
 
-int memblock_alloc_nid_checks(void)
+static int memblock_alloc_nid_checks_internal(int flags)
 {
-	const char *func_testing = "memblock_alloc_try_nid";
+	const char *func = get_func_testing(flags);
 
+	alloc_nid_test_flags = flags;
 	prefix_reset();
-	prefix_push(func_testing);
-	test_print("Running %s tests...\n", func_testing);
+	prefix_push(func);
+	test_print("Running %s tests...\n", func);
 
 	reset_memblock_attributes();
 	dummy_physical_memory_init();

@@ -1225,3 +1267,11 @@ int memblock_alloc_nid_checks(void)
 
 	return 0;
 }
+
+int memblock_alloc_nid_checks(void)
+{
+	memblock_alloc_nid_checks_internal(TEST_ZEROED);
+	memblock_alloc_nid_checks_internal(TEST_RAW);
+
+	return 0;
+}
Signature · 1 line
2.25.1
#14 lore raw

Re: [PATCH v2 6/8] memblock tests: update alloc_nid_api to test memblock_alloc_try_nid_raw

Quoted 27 lines · Rebecca Mckeever

Update memblock_alloc_try_nid() tests so that they test either memblock_alloc_try_nid() or memblock_alloc_try_nid_raw() depending on the value of alloc_nid_test_flags. Run through all the existing tests in alloc_nid_api twice: once for memblock_alloc_try_nid() and once for memblock_alloc_try_nid_raw().

When the tests run memblock_alloc_try_nid(), they test that the entire memory region is zero. When the tests run memblock_alloc_try_nid_raw(), they test that the entire memory region is nonzero.

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/alloc_nid_api.c | 188 ++++++++++++-------
 1 file changed, 119 insertions(+), 69 deletions(-)
tools/testing/memblock/tests/alloc_nid_api.c +4 -0
index 82fa8ea36320..2c1d5035e057 100644
@@ -1,6 +1,34 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include "alloc_nid_api.h"
 
+static const char * const func_testing[] = {
+	"memblock_alloc_try_nid",
+	"memblock_alloc_try_nid_raw"
+};

tools/testing/memblock/tests/alloc_nid_api.c line 17

Quoted 10 lines
tools/testing/memblock/tests/alloc_nid_api.c +10 -0
+
+static int alloc_nid_test_flags = TEST_ZEROED;
+
+static inline const char * const get_func_testing(int flags)
+{
+	if (flags & TEST_RAW)
+		return func_testing[1];
+	else
+		return func_testing[0];
+}

Same comments as for patch #5. Otherwise looks good.

Signature · 3 lines
Thanks,

David / dhildenb
author
#15 lore raw

PATCH v2 7/8 memblock tests: add tests for memblock_*bottom_up functions

In reply to Rebecca Mckeever

Add simple tests for memblock_set_bottom_up() and memblock_bottom_up().

Reviewed-by David Hildenbrand <[email protected]>
Reviewed-by Shaoqin Huang <[email protected]>
Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/basic_api.c | 45 ++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
tools/testing/memblock/tests/basic_api.c +45 -0
index ea79396e4611..c7490291c485 100644
@@ -1679,6 +1679,50 @@ static int memblock_free_checks(void)
 	return 0;
 }
 
+static int memblock_set_bottom_up_check(void)
+{
+	prefix_push("memblock_set_bottom_up");
+
+	memblock_set_bottom_up(false);
+	ASSERT_EQ(memblock.bottom_up, false);
+	memblock_set_bottom_up(true);
+	ASSERT_EQ(memblock.bottom_up, true);
+
+	reset_memblock_attributes();
+	test_pass_pop();
+
+	return 0;
+}
+
+static int memblock_bottom_up_check(void)
+{
+	prefix_push("memblock_bottom_up");
+
+	memblock_set_bottom_up(false);
+	ASSERT_EQ(memblock_bottom_up(), memblock.bottom_up);
+	ASSERT_EQ(memblock_bottom_up(), false);
+	memblock_set_bottom_up(true);
+	ASSERT_EQ(memblock_bottom_up(), memblock.bottom_up);
+	ASSERT_EQ(memblock_bottom_up(), true);
+
+	reset_memblock_attributes();
+	test_pass_pop();
+
+	return 0;
+}
+
+static int memblock_bottom_up_checks(void)
+{
+	test_print("Running memblock_*bottom_up tests...\n");
+
+	prefix_reset();
+	memblock_set_bottom_up_check();
+	prefix_reset();
+	memblock_bottom_up_check();
+
+	return 0;
+}
+
 int memblock_basic_checks(void)
 {
 	memblock_initialization_check();

@@ -1686,6 +1730,7 @@ int memblock_basic_checks(void)
 	memblock_reserve_checks();
 	memblock_remove_checks();
 	memblock_free_checks();
+	memblock_bottom_up_checks();
 
 	return 0;
 }
Signature · 1 line
2.25.1
author
#16 lore raw

PATCH v2 8/8 memblock tests: add tests for memblock_trim_memory

In reply to Rebecca Mckeever

Add tests for memblock_trim_memory() for the following scenarios:
- all regions aligned
- one region unalign that is smaller than the alignment
- one region unaligned at the base
- one region unaligned at the end

Signed-off-by Rebecca Mckeever <[email protected]>
---
 tools/testing/memblock/tests/basic_api.c | 223 +++++++++++++++++++++++
 1 file changed, 223 insertions(+)
tools/testing/memblock/tests/basic_api.c +223 -0
index c7490291c485..7048319bb096 100644
@@ -8,6 +8,7 @@
 #define FUNC_RESERVE					"memblock_reserve"
 #define FUNC_REMOVE					"memblock_remove"
 #define FUNC_FREE					"memblock_free"
+#define FUNC_TRIM					"memblock_trim_memory"
 
 static int memblock_initialization_check(void)
 {

@@ -1723,6 +1724,227 @@ static int memblock_bottom_up_checks(void)
 	return 0;
 }
 
+/*
+ * A test that tries to trim memory when both ends of the memory region are
+ * aligned. Expect that the memory will not be trimmed. Expect the counter to
+ * not be updated.
+ */
+static int memblock_trim_memory_aligned_check(void)
+{
+	struct memblock_region *rgn;
+	phys_addr_t alignment = SMP_CACHE_BYTES;
+
+	rgn = &memblock.memory.regions[0];
+
+	struct region r = {
+		.base = alignment,
+		.size = alignment * 4
+	};
+
+	PREFIX_PUSH();
+
+	reset_memblock_regions();
+	memblock_add(r.base, r.size);
+	memblock_trim_memory(alignment);
+
+	ASSERT_EQ(rgn->base, r.base);
+	ASSERT_EQ(rgn->size, r.size);
+
+	ASSERT_EQ(memblock.memory.cnt, 1);
+
+	test_pass_pop();
+
+	return 0;
+}
+
+/*
+ * A test that tries to trim memory when there are two available regions, r1 and
+ * r2. Region r1 is aligned on both ends and region r2 is unaligned on one end
+ * and smaller than the alignment:
+ *
+ *                                     alignment
+ *                                     |--------|
+ * |        +-----------------+        +------+   |
+ * |        |        r1       |        |  r2  |   |
+ * +--------+-----------------+--------+------+---+
+ *          ^        ^        ^        ^      ^
+ *          |________|________|________|      |
+ *                            |               Unaligned address
+ *                Aligned addresses
+ *
+ * Expect that r1 will not be trimmed and r2 will be removed. Expect the
+ * counter to be updated.
+ */
+static int memblock_trim_memory_too_small_check(void)
+{
+	struct memblock_region *rgn;
+	phys_addr_t alignment = SMP_CACHE_BYTES;
+
+	rgn = &memblock.memory.regions[0];
+
+	struct region r1 = {
+		.base = alignment,
+		.size = alignment * 2
+	};
+	struct region r2 = {
+		.base = alignment * 4,
+		.size = alignment - SZ_2
+	};
+
+	PREFIX_PUSH();
+
+	reset_memblock_regions();
+	memblock_add(r1.base, r1.size);
+	memblock_add(r2.base, r2.size);
+	memblock_trim_memory(alignment);
+
+	ASSERT_EQ(rgn->base, r1.base);
+	ASSERT_EQ(rgn->size, r1.size);
+
+	ASSERT_EQ(memblock.memory.cnt, 1);
+
+	test_pass_pop();
+
+	return 0;
+}
+
+/*
+ * A test that tries to trim memory when there are two available regions, r1 and
+ * r2. Region r1 is aligned on both ends and region r2 is unaligned at the base
+ * and aligned at the end:
+ *
+ *                               Unaligned address
+ *                                       |
+ *                                       v
+ * |        +-----------------+          +---------------+   |
+ * |        |        r1       |          |      r2       |   |
+ * +--------+-----------------+----------+---------------+---+
+ *          ^        ^        ^        ^        ^        ^
+ *          |________|________|________|________|________|
+ *                            |
+ *                    Aligned addresses
+ *
+ * Expect that r1 will not be trimmed and r2 will be trimmed at the base.
+ * Expect the counter to not be updated.
+ */
+static int memblock_trim_memory_unaligned_base_check(void)
+{
+	struct memblock_region *rgn1, *rgn2;
+	phys_addr_t alignment = SMP_CACHE_BYTES;
+	phys_addr_t offset = SZ_2;
+	phys_addr_t new_r2_base, new_r2_size;
+
+	rgn1 = &memblock.memory.regions[0];
+	rgn2 = &memblock.memory.regions[1];
+
+	struct region r1 = {
+		.base = alignment,
+		.size = alignment * 2
+	};
+	struct region r2 = {
+		.base = alignment * 4 + offset,
+		.size = alignment * 2 - offset
+	};
+
+	PREFIX_PUSH();
+
+	new_r2_base = r2.base + (alignment - offset);
+	new_r2_size = r2.size - (alignment - offset);
+
+	reset_memblock_regions();
+	memblock_add(r1.base, r1.size);
+	memblock_add(r2.base, r2.size);
+	memblock_trim_memory(alignment);
+
+	ASSERT_EQ(rgn1->base, r1.base);
+	ASSERT_EQ(rgn1->size, r1.size);
+
+	ASSERT_EQ(rgn2->base, new_r2_base);
+	ASSERT_EQ(rgn2->size, new_r2_size);
+
+	ASSERT_EQ(memblock.memory.cnt, 2);
+
+	test_pass_pop();
+
+	return 0;
+}
+
+/*
+ * A test that tries to trim memory when there are two available regions, r1 and
+ * r2. Region r1 is aligned on both ends and region r2 is aligned at the base
+ * and unaligned at the end:
+ *
+ *                                             Unaligned address
+ *                                                     |
+ *                                                     v
+ * |        +-----------------+        +---------------+   |
+ * |        |        r1       |        |      r2       |   |
+ * +--------+-----------------+--------+---------------+---+
+ *          ^        ^        ^        ^        ^        ^
+ *          |________|________|________|________|________|
+ *                            |
+ *                    Aligned addresses
+ *
+ * Expect that r1 will not be trimmed and r2 will be trimmed at the base.
+ * Expect the counter to not be updated.
+ */
+static int memblock_trim_memory_unaligned_end_check(void)
+{
+	struct memblock_region *rgn1, *rgn2;
+	phys_addr_t alignment = SMP_CACHE_BYTES;
+	phys_addr_t offset = SZ_2;
+	phys_addr_t new_r2_size;
+
+	rgn1 = &memblock.memory.regions[0];
+	rgn2 = &memblock.memory.regions[1];
+
+	struct region r1 = {
+		.base = alignment,
+		.size = alignment * 2
+	};
+	struct region r2 = {
+		.base = alignment * 4,
+		.size = alignment * 2 - offset
+	};
+
+	PREFIX_PUSH();
+
+	new_r2_size = r2.size - (alignment - offset);
+
+	reset_memblock_regions();
+	memblock_add(r1.base, r1.size);
+	memblock_add(r2.base, r2.size);
+	memblock_trim_memory(alignment);
+
+	ASSERT_EQ(rgn1->base, r1.base);
+	ASSERT_EQ(rgn1->size, r1.size);
+
+	ASSERT_EQ(rgn2->base, r2.base);
+	ASSERT_EQ(rgn2->size, new_r2_size);
+
+	ASSERT_EQ(memblock.memory.cnt, 2);
+
+	test_pass_pop();
+
+	return 0;
+}
+
+static int memblock_trim_memory_checks(void)
+{
+	prefix_reset();
+	prefix_push(FUNC_TRIM);
+	test_print("Running %s tests...\n", FUNC_TRIM);
+
+	memblock_trim_memory_aligned_check();
+	memblock_trim_memory_too_small_check();
+	memblock_trim_memory_unaligned_base_check();
+	memblock_trim_memory_unaligned_end_check();
+
+	prefix_pop();
+
+	return 0;
+}
+
 int memblock_basic_checks(void)
 {
 	memblock_initialization_check();

@@ -1731,6 +1953,7 @@ int memblock_basic_checks(void)
 	memblock_remove_checks();
 	memblock_free_checks();
 	memblock_bottom_up_checks();
+	memblock_trim_memory_checks();
 
 	return 0;
 }
Signature · 1 line
2.25.1
#17 lore raw

Re: [PATCH v2 8/8] memblock tests: add tests for memblock_trim_memory

Quoted 3 lines · Rebecca Mckeever

Add tests for memblock_trim_memory() for the following scenarios:
- all regions aligned
- one region unalign that is smaller than the alignment

s/region unalign/unaligned region/

Quoted 2 lines

- one region unaligned at the base
- one region unaligned at the end

"unaligned region" ?

[...]

tools/testing/memblock/tests/basic_api.c static int memblock_bottom_up_checks(void) · line 1735

Quoted 10 lines
tools/testing/memblock/tests/basic_api.c +9 -0
+/*
+ * A test that tries to trim memory when both ends of the memory region are
+ * aligned. Expect that the memory will not be trimmed. Expect the counter to
+ * not be updated.
+ */
+static int memblock_trim_memory_aligned_check(void)
+{
+	struct memblock_region *rgn;
+	phys_addr_t alignment = SMP_CACHE_BYTES;

Could make that "const" -- same applies to other functions.

tools/testing/memblock/tests/basic_api.c static int memblock_bottom_up_checks(void) · line 1743

Quoted 8 lines
tools/testing/memblock/tests/basic_api.c +8 -0
+
+	rgn = &memblock.memory.regions[0];
+
+	struct region r = {
+		.base = alignment,
+		.size = alignment * 4
+	};
+

[[[.]

tools/testing/memblock/tests/basic_api.c static int memblock_bottom_up_checks(void) · line 1936

Quoted 5 lines
tools/testing/memblock/tests/basic_api.c +5 -0
+static int memblock_trim_memory_checks(void)
+{
+	prefix_reset();
+	prefix_push(FUNC_TRIM);
+	test_print("Running %s tests...\n", FUNC_TRIM);

Just a note that this could have been

test_print("Running " FUNC_TRIM " tests...\n");

But it's good to keep this consistent for all tests.

Nice test cases!

Signature · 3 lines
Thanks,

David / dhildenb