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

From: Huang, Shaoqin
Date: Tue Aug 16 2022 - 03:01:11 EST



On 8/16/2022 10:15 AM, Rebecca Mckeever wrote:
On Mon, Aug 15, 2022 at 04:50:51PM +0800, Huang, Shaoqin wrote:
Hi, Rebecca.

On 8/14/2022 1:53 PM, Rebecca Mckeever wrote:
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().

Signed-off-by: Rebecca Mckeever <remckee0@xxxxxxxxx>
---
tools/testing/memblock/tests/alloc_api.c | 24 ++++++++++++++++++++++++
tools/testing/memblock/tests/common.c | 7 +++++++
tools/testing/memblock/tests/common.h | 13 +++++++++++++
3 files changed, 44 insertions(+)

diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
index a14f38eb8a89..71c89cb9b2a8 100644
--- a/tools/testing/memblock/tests/alloc_api.c
+++ b/tools/testing/memblock/tests/alloc_api.c
@@ -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());
@@ -442,6 +456,7 @@ static int alloc_no_memory_generic_check(void)
PREFIX_PUSH();
reset_memblock_regions();
+ fill_memblock();

Maybe we don't need this line, it has no effect at here. Anyway, Others
Looks Good to me.

Do you mean that it's not necessary since the test doesn't run
verify_mem_content()?


Yes. And the test function name is alloc_no_memory_...(). So I just think no memory operation in it may be same with the function name.

Reviewed-by: Shaoqin Huang <shaoqin.huang@xxxxxxxxx>

allocated_ptr = memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
@@ -472,6 +487,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 +545,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 +589,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 +638,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 +691,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);
diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
index 76a8ad818f3a..0ca26fe12c38 100644
--- a/tools/testing/memblock/tests/common.c
+++ b/tools/testing/memblock/tests/common.c
@@ -60,16 +60,23 @@ void reset_memblock_attributes(void)
memblock.current_limit = MEMBLOCK_ALLOC_ANYWHERE;
}
+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)
diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
index d396e5423a8e..7a16a7dc8f2c 100644
--- a/tools/testing/memblock/tests/common.h
+++ b/tools/testing/memblock/tests/common.h
@@ -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__)
/*
@@ -70,6 +82,7 @@ struct region {
void reset_memblock_regions(void);
void reset_memblock_attributes(void);
+void fill_memblock(void);
void setup_memblock(void);
void dummy_physical_memory_init(void);
void dummy_physical_memory_cleanup(void);