These patches update the style of comments for memblock_add_*(),
memblock_reserve_*(), memblock_remove_*(), and memblock_free_*()
functions to match the style of comments in tests/alloc_*.c and
remove extra spaces in those comments to conform to Linux kernel
coding style. This completes an item from the TODO list.
Rebecca Mckeever (5):
memblock tests: update style of comments for memblock_add_*()
functions
memblock tests: update style of comments for memblock_reserve_*()
functions
memblock tests: update style of comments for memblock_remove_*()
functions
memblock tests: update style of comments for memblock_free_*()
functions
memblock tests: remove completed TODO item
tools/testing/memblock/TODO | 3 -
tools/testing/memblock/tests/basic_api.c | 350 +++++++++++++++++------
2 files changed, 256 insertions(+), 97 deletions(-)
---
Changes
v1 -> v2
Add a comma in PATCH 1
Move separator outside block name in PATCH 1
Squash whitespace edit into PATCH 2
Add a colon before memory block visuals in PATCH 1 and PATCH 2
Add PATCH 3 and PATCH 4
v2 -> v3
Add TODO edit (PATCH 5)
Add changes summary
---
--
2.32.0
Update comments in memblock_remove_*() functions to match the style used
in tests/alloc_*.c by rewording to make the expected outcome more apparent
and, if more than one memblock is involved, adding a visual of the
memory blocks.
If the comment has an extra column of spaces, remove the extra space at
the beginning of each line for consistency and to conform to Linux kernel
coding style.
Signed-off-by: Rebecca Mckeever <[email protected]>
---
tools/testing/memblock/tests/basic_api.c | 101 +++++++++++++++++------
1 file changed, 75 insertions(+), 26 deletions(-)
diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index 75cd7479ee54..08847dc5065e 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -550,14 +550,21 @@ static int memblock_reserve_checks(void)
return 0;
}
- /*
- * A simple test that tries to remove the first entry of the array of
- * available memory regions. By "removing" a region we mean overwriting it
- * with the next region in memblock.memory. To check this is the case, the
- * test adds two memory blocks and verifies that the value of the latter
- * was used to erase r1 region. It also checks if the region counter and
- * total size were updated to expected values.
- */
+/*
+ * A simple test that tries to remove the first entry of the array of
+ * with the next region in memblock.memory:
+ *
+ * | ...... +----------------+ |
+ * | : r1 : | r2 | |
+ * +--+----+----------+----------------+--+
+ * ^
+ * |
+ * rgn.base
+ *
+ * Expect to add two memory blocks r1 and r2 and then remove r1 region
+ * so that r2 is the first available region. The region counter and
+ * total size are updated.
+ */
static int memblock_remove_simple_check(void)
{
struct memblock_region *rgn;
@@ -587,11 +594,22 @@ static int memblock_remove_simple_check(void)
return 0;
}
- /*
- * A test that tries to remove a region that was not registered as available
- * memory (i.e. has no corresponding entry in memblock.memory). It verifies
- * that array, regions counter and total size were not modified.
- */
+/*
+ * A test that tries to remove a region that was not registered as available
+ * memory (i.e. has no corresponding entry in memblock.memory):
+ *
+ * +----------------+
+ * | r2 |
+ * +----------------+
+ * | +----+ |
+ * | | r1 | |
+ * +--+----+------------------------------+
+ * ^
+ * |
+ * rgn.base
+ *
+ * Expect the array, regions counter and total size to not be modified.
+ */
static int memblock_remove_absent_check(void)
{
struct memblock_region *rgn;
@@ -622,10 +640,21 @@ static int memblock_remove_absent_check(void)
/*
* A test that tries to remove a region which overlaps with the beginning of
- * the already existing entry r1 (that is r1.base < r2.base + r2.size). It
- * checks if only the intersection of both regions is removed from the available
- * memory pool. The test also checks if the regions counter and total size are
- * updated to expected values.
+ * the already existing entry r1 (that is r1.base < r2.base + r2.size):
+ *
+ * +-----------------+
+ * | r2 |
+ * +-----------------+
+ * | .........+--------+ |
+ * | : r1 | rgn | |
+ * +-----------------+--------+--------+--+
+ * ^ ^
+ * | |
+ * | rgn.base
+ * r1.base
+ *
+ * Expect that only the intersection of both regions is removed from the
+ * available memory pool. The regions counter and total size are updated.
*/
static int memblock_remove_overlap_top_check(void)
{
@@ -662,10 +691,20 @@ static int memblock_remove_overlap_top_check(void)
/*
* A test that tries to remove a region which overlaps with the end of the
- * first entry (that is r2.base < r1.base + r1.size). It checks if only the
- * intersection of both regions is removed from the available memory pool.
- * The test also checks if the regions counter and total size are updated to
- * expected values.
+ * first entry (that is r2.base < r1.base + r1.size):
+ *
+ * +--------------------------------+
+ * | r2 |
+ * +--------------------------------+
+ * | +---+..... |
+ * | |rgn| r1 : |
+ * +-+---+----+---------------------------+
+ * ^
+ * |
+ * r1.base
+ *
+ * Expect that only the intersection of both regions is removed from the
+ * available memory pool. The regions counter and total size are updated.
*/
static int memblock_remove_overlap_bottom_check(void)
{
@@ -700,11 +739,21 @@ static int memblock_remove_overlap_bottom_check(void)
/*
* A test that tries to remove a region which is within the range of the
* already existing entry (that is
- * (r1.base < r2.base) && (r2.base + r2.size < r1.base + r1.size)).
- * It checks if the region is split into two - one that ends at r2.base and
- * second that starts at r2.base + size, with appropriate sizes. The test
- * also checks if the region counter and total size were updated to
- * expected values.
+ * (r1.base < r2.base) && (r2.base + r2.size < r1.base + r1.size)):
+ *
+ * +----+
+ * | r2 |
+ * +----+
+ * | +-------------+....+---------------+ |
+ * | | rgn1 | r1 | rgn2 | |
+ * +-+-------------+----+---------------+-+
+ * ^
+ * |
+ * r1.base
+ *
+ * Expect that the region is split into two - one that ends at r2.base and
+ * another that starts at r2.base + size, with appropriate sizes. The
+ * region counter and total size are updated.
*/
static int memblock_remove_within_check(void)
{
--
2.32.0
Update comments in memblock_add_*() functions to match the style used
in tests/alloc_*.c by rewording to make the expected outcome more apparent
and, if more than one memblock is involved, adding a visual of the
memory blocks.
Signed-off-by: Rebecca Mckeever <[email protected]>
---
tools/testing/memblock/tests/basic_api.c | 77 +++++++++++++++++-------
1 file changed, 55 insertions(+), 22 deletions(-)
diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index fbc1ce160303..e96be0319ad4 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -26,8 +26,8 @@ static int memblock_initialization_check(void)
/*
* A simple test that adds a memory block of a specified base address
* and size to the collection of available memory regions (memblock.memory).
- * It checks if a new entry was created and if region counter and total memory
- * were correctly updated.
+ * Expect to create a new entry. The region counter and total memory get
+ * updated.
*/
static int memblock_add_simple_check(void)
{
@@ -53,10 +53,10 @@ static int memblock_add_simple_check(void)
}
/*
- * A simple test that adds a memory block of a specified base address, size
+ * A simple test that adds a memory block of a specified base address, size,
* NUMA node and memory flags to the collection of available memory regions.
- * It checks if the new entry, region counter and total memory size have
- * expected values.
+ * Expect to create a new entry. The region counter and total memory get
+ * updated.
*/
static int memblock_add_node_simple_check(void)
{
@@ -87,9 +87,15 @@ static int memblock_add_node_simple_check(void)
/*
* A test that tries to add two memory blocks that don't overlap with one
- * another. It checks if two correctly initialized entries were added to the
- * collection of available memory regions (memblock.memory) and if this
- * change was reflected in memblock.memory's total size and region counter.
+ * another:
+ *
+ * | +--------+ +--------+ |
+ * | | r1 | | r2 | |
+ * +--------+--------+--------+--------+--+
+ *
+ * Expect to add two correctly initialized entries to the collection of
+ * available memory regions (memblock.memory). The total size and
+ * region counter fields get updated.
*/
static int memblock_add_disjoint_check(void)
{
@@ -125,10 +131,20 @@ static int memblock_add_disjoint_check(void)
/*
* A test that tries to add two memory blocks, where the second one overlaps
- * with the beginning of the first entry (that is r1.base < r2.base + r2.size).
- * After this, it checks if two entries are merged into one region that starts
- * at r2.base and has size of two regions minus their intersection. It also
- * verifies the reported total size of the available memory and region counter.
+ * with the beginning of the first entry (that is r1.base < r2.base + r2.size):
+ *
+ * | +----+----+------------+ |
+ * | | |r2 | r1 | |
+ * +----+----+----+------------+----------+
+ * ^ ^
+ * | |
+ * | r1.base
+ * |
+ * r2.base
+ *
+ * Expect to merge the two entries into one region that starts at r2.base
+ * and has size of two regions minus their intersection. The total size of
+ * the available memory is updated, and the region counter stays the same.
*/
static int memblock_add_overlap_top_check(void)
{
@@ -163,11 +179,20 @@ static int memblock_add_overlap_top_check(void)
/*
* A test that tries to add two memory blocks, where the second one overlaps
- * with the end of the first entry (that is r2.base < r1.base + r1.size).
- * After this, it checks if two entries are merged into one region that starts
- * at r1.base and has size of two regions minus their intersection. It verifies
- * that memblock can still see only one entry and has a correct total size of
- * the available memory.
+ * with the end of the first entry (that is r2.base < r1.base + r1.size):
+ *
+ * | +--+------+----------+ |
+ * | | | r1 | r2 | |
+ * +--+--+------+----------+--------------+
+ * ^ ^
+ * | |
+ * | r2.base
+ * |
+ * r1.base
+ *
+ * Expect to merge the two entries into one region that starts at r1.base
+ * and has size of two regions minus their intersection. The total size of
+ * the available memory is updated, and the region counter stays the same.
*/
static int memblock_add_overlap_bottom_check(void)
{
@@ -203,9 +228,17 @@ static int memblock_add_overlap_bottom_check(void)
/*
* A test that tries to add two memory blocks, where the second one is
* within the range of the first entry (that is r1.base < r2.base &&
- * r2.base + r2.size < r1.base + r1.size). It checks if two entries are merged
- * into one region that stays the same. The counter and total size of available
- * memory are expected to not be updated.
+ * r2.base + r2.size < r1.base + r1.size):
+ *
+ * | +-------+--+-----------------------+
+ * | | |r2| r1 |
+ * +---+-------+--+-----------------------+
+ * ^
+ * |
+ * r1.base
+ *
+ * Expect to merge two entries into one region that stays the same.
+ * The counter and total size of available memory are not updated.
*/
static int memblock_add_within_check(void)
{
@@ -236,8 +269,8 @@ static int memblock_add_within_check(void)
}
/*
- * A simple test that tries to add the same memory block twice. The counter
- * and total size of available memory are expected to not be updated.
+ * A simple test that tries to add the same memory block twice. Expect
+ * the counter and total size of available memory to not be updated.
*/
static int memblock_add_twice_check(void)
{
--
2.32.0
Update comments in memblock_free_*() functions to match the style used
in tests/alloc_*.c by rewording to make the expected outcome more apparent
and, if more than one memblock is involved, adding a visual of the
memory blocks.
If the comment has an extra column of spaces, remove the extra space at
the beginning of each line for consistency and to conform to Linux kernel
coding style.
Signed-off-by: Rebecca Mckeever <[email protected]>
---
tools/testing/memblock/tests/basic_api.c | 90 ++++++++++++++++++------
1 file changed, 69 insertions(+), 21 deletions(-)
diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
index 08847dc5065e..f646350d370e 100644
--- a/tools/testing/memblock/tests/basic_api.c
+++ b/tools/testing/memblock/tests/basic_api.c
@@ -806,10 +806,17 @@ static int memblock_remove_checks(void)
/*
* A simple test that tries to free a memory block that was marked earlier
* as reserved. By "freeing" a region we mean overwriting it with the next
- * entry in memblock.reserved. To check this is the case, the test reserves
- * two memory regions and verifies that the value of the latter was used to
- * erase r1 region.
- * The test also checks if the region counter and total size were updated.
+ * entry in memblock.reserved:
+ *
+ * | ...... +----+ |
+ * | : r1 : | r2 | |
+ * +--------------+----+-----------+----+-+
+ * ^
+ * |
+ * rgn.base
+ *
+ * Expect to reserve two memory regions and then erase r1 region with the
+ * value of r2. The region counter and total size are updated.
*/
static int memblock_free_simple_check(void)
{
@@ -840,11 +847,22 @@ static int memblock_free_simple_check(void)
return 0;
}
- /*
- * A test that tries to free a region that was not marked as reserved
- * (i.e. has no corresponding entry in memblock.reserved). It verifies
- * that array, regions counter and total size were not modified.
- */
+/*
+ * A test that tries to free a region that was not marked as reserved
+ * (i.e. has no corresponding entry in memblock.reserved):
+ *
+ * +----------------+
+ * | r2 |
+ * +----------------+
+ * | +----+ |
+ * | | r1 | |
+ * +--+----+------------------------------+
+ * ^
+ * |
+ * rgn.base
+ *
+ * The array, regions counter and total size are not modified.
+ */
static int memblock_free_absent_check(void)
{
struct memblock_region *rgn;
@@ -875,10 +893,22 @@ static int memblock_free_absent_check(void)
/*
* A test that tries to free a region which overlaps with the beginning of
- * the already existing entry r1 (that is r1.base < r2.base + r2.size). It
- * checks if only the intersection of both regions is freed. The test also
- * checks if the regions counter and total size are updated to expected
- * values.
+ * the already existing entry r1 (that is r1.base < r2.base + r2.size):
+ *
+ * +----+
+ * | r2 |
+ * +----+
+ * | ...+--------------+ |
+ * | : | r1 | |
+ * +----+--+--------------+---------------+
+ * ^ ^
+ * | |
+ * | rgn.base
+ * |
+ * r1.base
+ *
+ * Expect that only the intersection of both regions is freed. The
+ * regions counter and total size are updated.
*/
static int memblock_free_overlap_top_check(void)
{
@@ -913,9 +943,17 @@ static int memblock_free_overlap_top_check(void)
/*
* A test that tries to free a region which overlaps with the end of the
- * first entry (that is r2.base < r1.base + r1.size). It checks if only the
- * intersection of both regions is freed. The test also checks if the
- * regions counter and total size are updated to expected values.
+ * first entry (that is r2.base < r1.base + r1.size):
+ *
+ * +----------------+
+ * | r2 |
+ * +----------------+
+ * | +-----------+..... |
+ * | | r1 | : |
+ * +----+-----------+----+----------------+
+ *
+ * Expect that only the intersection of both regions is freed. The
+ * regions counter and total size are updated.
*/
static int memblock_free_overlap_bottom_check(void)
{
@@ -951,11 +989,21 @@ static int memblock_free_overlap_bottom_check(void)
/*
* A test that tries to free a region which is within the range of the
* already existing entry (that is
- * (r1.base < r2.base) && (r2.base + r2.size < r1.base + r1.size)).
- * It checks if the region is split into two - one that ends at r2.base and
- * second that starts at r2.base + size, with appropriate sizes. It is
- * expected that the region counter and total size fields were updated t
- * reflect that change.
+ * (r1.base < r2.base) && (r2.base + r2.size < r1.base + r1.size)):
+ *
+ * +----+
+ * | r2 |
+ * +----+
+ * | +------------+....+---------------+
+ * | | rgn1 | r1 | rgn2 |
+ * +----+------------+----+---------------+
+ * ^
+ * |
+ * r1.base
+ *
+ * Expect that the region is split into two - one that ends at r2.base and
+ * another that starts at r2.base + size, with appropriate sizes. The
+ * region counter and total size fields are updated.
*/
static int memblock_free_within_check(void)
{
--
2.32.0
Remove completed item from TODO list.
Signed-off-by: Rebecca Mckeever <[email protected]>
---
tools/testing/memblock/TODO | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tools/testing/memblock/TODO b/tools/testing/memblock/TODO
index c25b2fdec45e..cd1a30d5acc9 100644
--- a/tools/testing/memblock/TODO
+++ b/tools/testing/memblock/TODO
@@ -23,6 +23,3 @@ TODO
5. Add tests for memblock_alloc_node() to check if the correct NUMA node is set
for the new region
-
-6. Update comments in tests/basic_api.c to match the style used in
- tests/alloc_*.c
--
2.32.0
On 22.04.22 15:32, Rebecca Mckeever wrote:
> Update comments in memblock_add_*() functions to match the style used
> in tests/alloc_*.c by rewording to make the expected outcome more apparent
> and, if more than one memblock is involved, adding a visual of the
> memory blocks.
>
> Signed-off-by: Rebecca Mckeever <[email protected]>
Please make sure to not drop Rb/Ack tags and instead carry them along
when you resend patches. This, for example, makes it easier for
reviewers to spot what needs attention and review.
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 22.04.22 15:33, Rebecca Mckeever wrote:
> Update comments in memblock_remove_*() functions to match the style used
> in tests/alloc_*.c by rewording to make the expected outcome more apparent
> and, if more than one memblock is involved, adding a visual of the
> memory blocks.
>
> If the comment has an extra column of spaces, remove the extra space at
> the beginning of each line for consistency and to conform to Linux kernel
> coding style.
>
> Signed-off-by: Rebecca Mckeever <[email protected]>
> ---
> tools/testing/memblock/tests/basic_api.c | 101 +++++++++++++++++------
> 1 file changed, 75 insertions(+), 26 deletions(-)
>
> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
> index 75cd7479ee54..08847dc5065e 100644
> --- a/tools/testing/memblock/tests/basic_api.c
> +++ b/tools/testing/memblock/tests/basic_api.c
> @@ -550,14 +550,21 @@ static int memblock_reserve_checks(void)
> return 0;
> }
>
> - /*
> - * A simple test that tries to remove the first entry of the array of
> - * available memory regions. By "removing" a region we mean overwriting it
> - * with the next region in memblock.memory. To check this is the case, the
> - * test adds two memory blocks and verifies that the value of the latter
> - * was used to erase r1 region. It also checks if the region counter and
> - * total size were updated to expected values.
> - */
> +/*
> + * A simple test that tries to remove the first entry of the array of
> + * with the next region in memblock.memory:
I failed to parse this sentence "of the array of with".
> + *
> + * | ...... +----------------+ |
> + * | : r1 : | r2 | |
> + * +--+----+----------+----------------+--+
> + * ^
> + * |
> + * rgn.base
> + *
> + * Expect to add two memory blocks r1 and r2 and then remove r1 region
> + * so that r2 is the first available region. The region counter and
> + * total size are updated.
> + */
> static int memblock_remove_simple_check(void)
> {
> struct memblock_region *rgn;
> @@ -587,11 +594,22 @@ static int memblock_remove_simple_check(void)
> return 0;
> }
>
> - /*
> - * A test that tries to remove a region that was not registered as available
> - * memory (i.e. has no corresponding entry in memblock.memory). It verifies
> - * that array, regions counter and total size were not modified.
> - */
> +/*
> + * A test that tries to remove a region that was not registered as available
> + * memory (i.e. has no corresponding entry in memblock.memory):
> + *
> + * +----------------+
> + * | r2 |
> + * +----------------+
> + * | +----+ |
> + * | | r1 | |
> + * +--+----+------------------------------+
> + * ^
> + * |
> + * rgn.base
> + *
> + * Expect the array, regions counter and total size to not be modified.
> + */
> static int memblock_remove_absent_check(void)
> {
> struct memblock_region *rgn;
> @@ -622,10 +640,21 @@ static int memblock_remove_absent_check(void)
>
> /*
> * A test that tries to remove a region which overlaps with the beginning of
Wonder if it makes sense to mention r2 here. ("a region r2 ...") -- to
be precise, to always reference r1 and r2 accordingly instead of e.g.,
"the first entry". Same applies to the other function documentation you
touch in this patch.
--
Thanks,
David / dhildenb