2022-08-14 05:55:43

by Rebecca Mckeever

Subject: [PATCH 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().

Signed-off-by: Rebecca Mckeever <[email protected]>
---
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();

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

2022-08-15 09:04:26

by Huang, Shaoqin

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

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 <[email protected]>
> ---
> 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.

Reviewed-by: Shaoqin Huang <[email protected]>

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

2022-08-16 07:55:26

by Huang, Shaoqin

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

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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>
>>> 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);

2022-08-16 07:57:57

by Rebecca Mckeever

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

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 <[email protected]>
> > ---
> > 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()?

> Reviewed-by: Shaoqin Huang <[email protected]>
>
> > 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);