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

Rebecca Mckeever posted 8 patches 3 years, 7 months ago
There is a newer version of this series
[PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw
Posted by Rebecca Mckeever 3 years, 7 months ago
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 <remckee0@gmail.com>
---
 tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
 tools/testing/memblock/tests/common.h    | 25 ++++++
 2 files changed, 90 insertions(+), 33 deletions(-)

diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
index 65bff77dd55b..cf67687ae044 100644
--- a/tools/testing/memblock/tests/alloc_api.c
+++ b/tools/testing/memblock/tests/alloc_api.c
@@ -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;
+}
diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
index 58f84bf2c9ae..4fd3534ff955 100644
--- a/tools/testing/memblock/tests/common.h
+++ b/tools/testing/memblock/tests/common.h
@@ -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
-- 
2.25.1
Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw
Posted by David Hildenbrand 3 years, 7 months ago
On 19.08.22 10:34, Rebecca Mckeever wrote:
> 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).

> 
> Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> ---
>  tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
>  tools/testing/memblock/tests/common.h    | 25 ++++++
>  2 files changed, 90 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
> index 65bff77dd55b..cf67687ae044 100644
> --- a/tools/testing/memblock/tests/alloc_api.c
> +++ b/tools/testing/memblock/tests/alloc_api.c
> @@ -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];

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() ?


> diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
> index 58f84bf2c9ae..4fd3534ff955 100644
> --- a/tools/testing/memblock/tests/common.h
> +++ b/tools/testing/memblock/tests/common.h
> @@ -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.

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

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


-- 
Thanks,

David / dhildenb
Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw
Posted by Rebecca Mckeever 3 years, 7 months ago
On Tue, Aug 23, 2022 at 11:49:46AM +0200, David Hildenbrand wrote:
> On 19.08.22 10:34, Rebecca Mckeever wrote:
> > 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).
> 
> > 
> > Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> > ---
> >  tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
> >  tools/testing/memblock/tests/common.h    | 25 ++++++
> >  2 files changed, 90 insertions(+), 33 deletions(-)
> > 
> > diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
> > index 65bff77dd55b..cf67687ae044 100644
> > --- a/tools/testing/memblock/tests/alloc_api.c
> > +++ b/tools/testing/memblock/tests/alloc_api.c
> > @@ -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];
> 
> 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() ?
> 
> 
> > diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
> > index 58f84bf2c9ae..4fd3534ff955 100644
> > --- a/tools/testing/memblock/tests/common.h
> > +++ b/tools/testing/memblock/tests/common.h
> > @@ -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.
> 
Do you mean use two #defines instead of the enum? I thought enums were
preferred when defining related constants.

> > +
> >  /**
> >   * 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)
> 
> nit: why use verify here when the other functions "assert". I'd have
> called this something like "assert_mem_content()"
> 
> 
> -- 
> Thanks,
> 
> David / dhildenb
> 
> 
Thanks,
Rebecca
Re: [PATCH v2 5/8] memblock tests: update alloc_api to test memblock_alloc_raw
Posted by David Hildenbrand 3 years, 7 months ago
On 25.08.22 23:35, Rebecca Mckeever wrote:
> On Tue, Aug 23, 2022 at 11:49:46AM +0200, David Hildenbrand wrote:
>> On 19.08.22 10:34, Rebecca Mckeever wrote:
>>> 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).
>>
>>>
>>> Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
>>> ---
>>>  tools/testing/memblock/tests/alloc_api.c | 98 ++++++++++++++++--------
>>>  tools/testing/memblock/tests/common.h    | 25 ++++++
>>>  2 files changed, 90 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
>>> index 65bff77dd55b..cf67687ae044 100644
>>> --- a/tools/testing/memblock/tests/alloc_api.c
>>> +++ b/tools/testing/memblock/tests/alloc_api.c
>>> @@ -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];
>>
>> 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() ?
>>
>>
>>> diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
>>> index 58f84bf2c9ae..4fd3534ff955 100644
>>> --- a/tools/testing/memblock/tests/common.h
>>> +++ b/tools/testing/memblock/tests/common.h
>>> @@ -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.
>>
> 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 :)

-- 
Thanks,

David / dhildenb