[dpdk-dev] test/memzone: fix incorrect result judging criteria

Message ID 1517206986-32281-2-git-send-email-phil.yang@arm.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Phil Yang Jan. 29, 2018, 6:23 a.m. UTC
  While running DTS with ixgbe NIC, dpdk EAL will reserve some
memzones for the PCI device. So it is incorrect to judging the result by
only checking whether it has memzone remained.

Fixed by insert an unique name prefix into each memzone used in this case.
And verify whether it has used memzone remained at the end.

Fixes: 71330483a193 ("test/memzone: fix memory leak")

Signed-off-by: Phil Yang <phil.yang@arm.com>
---
 test/test/test_memzone.c | 141 +++++++++++++++++++++++++----------------------
 1 file changed, 74 insertions(+), 67 deletions(-)
  

Comments

Anatoly Burakov Jan. 29, 2018, 9:49 a.m. UTC | #1
On 29-Jan-18 6:23 AM, Phil Yang wrote:
> While running DTS with ixgbe NIC, dpdk EAL will reserve some
> memzones for the PCI device. So it is incorrect to judging the result by
> only checking whether it has memzone remained.
> 
> Fixed by insert an unique name prefix into each memzone used in this case.
> And verify whether it has used memzone remained at the end.
> 
> Fixes: 71330483a193 ("test/memzone: fix memory leak")
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> ---

Hi Phil,

I think this is a (rough) duplicate of what i've submitted recently:

http://dpdk.org/dev/patchwork/patch/34564/

I believe approach i've taken is a little cleaner and more generic :)
  

Patch

diff --git a/test/test/test_memzone.c b/test/test/test_memzone.c
index f6c9b56..2d42588 100644
--- a/test/test/test_memzone.c
+++ b/test/test/test_memzone.c
@@ -4,6 +4,7 @@ 
 
 #include <stdio.h>
 #include <stdint.h>
+#include <string.h>
 #include <inttypes.h>
 #include <sys/queue.h>
 
@@ -63,13 +64,13 @@  test_memzone_invalid_alignment(void)
 {
 	const struct rte_memzone * mz;
 
-	mz = rte_memzone_lookup("invalid_alignment");
+	mz = rte_memzone_lookup("UT_TMZ_invalid_alignment");
 	if (mz != NULL) {
 		printf("Zone with invalid alignment has been reserved\n");
 		return -1;
 	}
 
-	mz = rte_memzone_reserve_aligned("invalid_alignment", 100,
+	mz = rte_memzone_reserve_aligned("UT_TMZ_invalid_alignment", 100,
 			SOCKET_ID_ANY, 0, 100);
 	if (mz != NULL) {
 		printf("Zone with invalid alignment has been reserved\n");
@@ -83,14 +84,14 @@  test_memzone_reserving_zone_size_bigger_than_the_maximum(void)
 {
 	const struct rte_memzone * mz;
 
-	mz = rte_memzone_lookup("zone_size_bigger_than_the_maximum");
+	mz = rte_memzone_lookup("UT_TMZ_zone_size_bigger_than_the_maximum");
 	if (mz != NULL) {
 		printf("zone_size_bigger_than_the_maximum has been reserved\n");
 		return -1;
 	}
 
-	mz = rte_memzone_reserve("zone_size_bigger_than_the_maximum", (size_t)-1,
-			SOCKET_ID_ANY, 0);
+	mz = rte_memzone_reserve("UT_TMZ_zone_size_bigger_than_the_maximum",
+			(size_t)-1, SOCKET_ID_ANY, 0);
 	if (mz != NULL) {
 		printf("It is impossible to reserve such big a memzone\n");
 		return -1;
@@ -137,8 +138,8 @@  test_memzone_reserve_flags(void)
 	 * available page size (i.e 1GB ) when 2MB pages are unavailable.
 	 */
 	if (hugepage_2MB_avail) {
-		mz = rte_memzone_reserve("flag_zone_2M", size, SOCKET_ID_ANY,
-				RTE_MEMZONE_2MB);
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_2M", size,
+				SOCKET_ID_ANY, RTE_MEMZONE_2MB);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 2MB\n");
 			return -1;
@@ -152,7 +153,8 @@  test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_2M_HINT", size,
+				SOCKET_ID_ANY,
 				RTE_MEMZONE_2MB|RTE_MEMZONE_SIZE_HINT_ONLY);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 2MB\n");
@@ -171,7 +173,8 @@  test_memzone_reserve_flags(void)
 		 * HINT flag is indicated
 		 */
 		if (!hugepage_1GB_avail) {
-			mz = rte_memzone_reserve("flag_zone_1G_HINT", size, SOCKET_ID_ANY,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_1G_HINT",
+					size, SOCKET_ID_ANY,
 					RTE_MEMZONE_1GB|RTE_MEMZONE_SIZE_HINT_ONLY);
 			if (mz == NULL) {
 				printf("MEMZONE FLAG 1GB & HINT\n");
@@ -186,8 +189,8 @@  test_memzone_reserve_flags(void)
 				return -1;
 			}
 
-			mz = rte_memzone_reserve("flag_zone_1G", size, SOCKET_ID_ANY,
-					RTE_MEMZONE_1GB);
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_1G", size,
+					SOCKET_ID_ANY, RTE_MEMZONE_1GB);
 			if (mz != NULL) {
 				printf("MEMZONE FLAG 1GB\n");
 				return -1;
@@ -197,8 +200,8 @@  test_memzone_reserve_flags(void)
 
 	/*As with 2MB tests above for 1GB huge page requests*/
 	if (hugepage_1GB_avail) {
-		mz = rte_memzone_reserve("flag_zone_1G", size, SOCKET_ID_ANY,
-				RTE_MEMZONE_1GB);
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_1G", size,
+				SOCKET_ID_ANY, RTE_MEMZONE_1GB);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 1GB\n");
 			return -1;
@@ -212,7 +215,8 @@  test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		mz = rte_memzone_reserve("flag_zone_1G_HINT", size, SOCKET_ID_ANY,
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_1G_HINT",
+				size, SOCKET_ID_ANY,
 				RTE_MEMZONE_1GB|RTE_MEMZONE_SIZE_HINT_ONLY);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 1GB\n");
@@ -231,7 +235,8 @@  test_memzone_reserve_flags(void)
 		 * HINT flag is indicated
 		 */
 		if (!hugepage_2MB_avail) {
-			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_2M_HINT",
+					size, SOCKET_ID_ANY,
 					RTE_MEMZONE_2MB|RTE_MEMZONE_SIZE_HINT_ONLY);
 			if (mz == NULL){
 				printf("MEMZONE FLAG 2MB & HINT\n");
@@ -245,8 +250,8 @@  test_memzone_reserve_flags(void)
 				printf("Fail memzone free\n");
 				return -1;
 			}
-			mz = rte_memzone_reserve("flag_zone_2M", size, SOCKET_ID_ANY,
-					RTE_MEMZONE_2MB);
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_2M", size,
+					SOCKET_ID_ANY, RTE_MEMZONE_2MB);
 			if (mz != NULL) {
 				printf("MEMZONE FLAG 2MB\n");
 				return -1;
@@ -254,8 +259,9 @@  test_memzone_reserve_flags(void)
 		}
 
 		if (hugepage_2MB_avail && hugepage_1GB_avail) {
-			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
-								RTE_MEMZONE_2MB|RTE_MEMZONE_1GB);
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_2M_HINT",
+					size, SOCKET_ID_ANY,
+					RTE_MEMZONE_2MB|RTE_MEMZONE_1GB);
 			if (mz == NULL) {
 				printf("BOTH SIZES SET\n");
 				return -1;
@@ -279,8 +285,8 @@  test_memzone_reserve_flags(void)
 	 * page size (i.e 16GB ) when 16MB pages are unavailable.
 	 */
 	if (hugepage_16MB_avail) {
-		mz = rte_memzone_reserve("flag_zone_16M", size, SOCKET_ID_ANY,
-				RTE_MEMZONE_16MB);
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_16M", size,
+				SOCKET_ID_ANY, RTE_MEMZONE_16MB);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 16MB\n");
 			return -1;
@@ -294,7 +300,7 @@  test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_16M_HINT", size,
 		SOCKET_ID_ANY, RTE_MEMZONE_16MB|RTE_MEMZONE_SIZE_HINT_ONLY);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 16MB\n");
@@ -313,8 +319,8 @@  test_memzone_reserve_flags(void)
 		 * unless HINT flag is indicated
 		 */
 		if (!hugepage_16GB_avail) {
-			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
-				SOCKET_ID_ANY,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_16G_HINT",
+				size, SOCKET_ID_ANY,
 				RTE_MEMZONE_16GB|RTE_MEMZONE_SIZE_HINT_ONLY);
 			if (mz == NULL) {
 				printf("MEMZONE FLAG 16GB & HINT\n");
@@ -329,7 +335,7 @@  test_memzone_reserve_flags(void)
 				return -1;
 			}
 
-			mz = rte_memzone_reserve("flag_zone_16G", size,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_16G", size,
 				SOCKET_ID_ANY, RTE_MEMZONE_16GB);
 			if (mz != NULL) {
 				printf("MEMZONE FLAG 16GB\n");
@@ -339,8 +345,8 @@  test_memzone_reserve_flags(void)
 	}
 	/*As with 16MB tests above for 16GB huge page requests*/
 	if (hugepage_16GB_avail) {
-		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
-				RTE_MEMZONE_16GB);
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_16G", size,
+				SOCKET_ID_ANY, RTE_MEMZONE_16GB);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 16GB\n");
 			return -1;
@@ -354,7 +360,7 @@  test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
+		mz = rte_memzone_reserve("UT_TMZ_flag_zone_16G_HINT", size,
 		SOCKET_ID_ANY, RTE_MEMZONE_16GB|RTE_MEMZONE_SIZE_HINT_ONLY);
 		if (mz == NULL) {
 			printf("MEMZONE FLAG 16GB\n");
@@ -373,8 +379,8 @@  test_memzone_reserve_flags(void)
 		 * unless HINT flag is indicated
 		 */
 		if (!hugepage_16MB_avail) {
-			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
-				SOCKET_ID_ANY,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_16M_HINT",
+				size, SOCKET_ID_ANY,
 				RTE_MEMZONE_16MB|RTE_MEMZONE_SIZE_HINT_ONLY);
 			if (mz == NULL) {
 				printf("MEMZONE FLAG 16MB & HINT\n");
@@ -388,7 +394,7 @@  test_memzone_reserve_flags(void)
 				printf("Fail memzone free\n");
 				return -1;
 			}
-			mz = rte_memzone_reserve("flag_zone_16M", size,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_16M", size,
 				SOCKET_ID_ANY, RTE_MEMZONE_16MB);
 			if (mz != NULL) {
 				printf("MEMZONE FLAG 16MB\n");
@@ -397,8 +403,8 @@  test_memzone_reserve_flags(void)
 		}
 
 		if (hugepage_16MB_avail && hugepage_16GB_avail) {
-			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
-				SOCKET_ID_ANY,
+			mz = rte_memzone_reserve("UT_TMZ_flag_zone_16M_HINT",
+				size, SOCKET_ID_ANY,
 				RTE_MEMZONE_16MB|RTE_MEMZONE_16GB);
 			if (mz == NULL) {
 				printf("BOTH SIZES SET\n");
@@ -455,7 +461,7 @@  test_memzone_reserve_max(void)
 		return 0;
 	}
 
-	mz = rte_memzone_reserve("max_zone", 0, SOCKET_ID_ANY, 0);
+	mz = rte_memzone_reserve("UT_TMZ_max_zone", 0, SOCKET_ID_ANY, 0);
 	if (mz == NULL){
 		printf("Failed to reserve a big chunk of memory - %s\n",
 				rte_strerror(rte_errno));
@@ -497,7 +503,7 @@  test_memzone_reserve_max_aligned(void)
 		return 0;
 	}
 
-	mz = rte_memzone_reserve_aligned("max_zone_aligned", 0,
+	mz = rte_memzone_reserve_aligned("UT_TMZ_max_zone_aligned", 0,
 			SOCKET_ID_ANY, 0, align);
 	if (mz == NULL){
 		printf("Failed to reserve a big chunk of memory - %s\n",
@@ -535,24 +541,24 @@  test_memzone_aligned(void)
 	const struct rte_memzone *memzone_aligned_1024;
 
 	/* memzone that should automatically be adjusted to align on 64 bytes */
-	memzone_aligned_32 = rte_memzone_reserve_aligned("aligned_32", 100,
-				SOCKET_ID_ANY, 0, 32);
+	memzone_aligned_32 = rte_memzone_reserve_aligned("UT_TMZ_aligned_32",
+			100, SOCKET_ID_ANY, 0, 32);
 
 	/* memzone that is supposed to be aligned on a 128 byte boundary */
-	memzone_aligned_128 = rte_memzone_reserve_aligned("aligned_128", 100,
-				SOCKET_ID_ANY, 0, 128);
+	memzone_aligned_128 = rte_memzone_reserve_aligned("UT_TMZ_aligned_128",
+			100, SOCKET_ID_ANY, 0, 128);
 
 	/* memzone that is supposed to be aligned on a 256 byte boundary */
-	memzone_aligned_256 = rte_memzone_reserve_aligned("aligned_256", 100,
-				SOCKET_ID_ANY, 0, 256);
+	memzone_aligned_256 = rte_memzone_reserve_aligned("UT_TMZ_aligned_256",
+			100, SOCKET_ID_ANY, 0, 256);
 
 	/* memzone that is supposed to be aligned on a 512 byte boundary */
-	memzone_aligned_512 = rte_memzone_reserve_aligned("aligned_512", 100,
-				SOCKET_ID_ANY, 0, 512);
+	memzone_aligned_512 = rte_memzone_reserve_aligned("UT_TMZ_aligned_512",
+			100, SOCKET_ID_ANY, 0, 512);
 
 	/* memzone that is supposed to be aligned on a 1024 byte boundary */
-	memzone_aligned_1024 = rte_memzone_reserve_aligned("aligned_1024", 100,
-				SOCKET_ID_ANY, 0, 1024);
+	memzone_aligned_1024 = rte_memzone_reserve_aligned("UT_TMZ_aligned_1024",
+			100, SOCKET_ID_ANY, 0, 1024);
 
 	printf("check alignments and lengths\n");
 	if (memzone_aligned_32 == NULL) {
@@ -725,7 +731,7 @@  test_memzone_bounded(void)
 	int rc;
 
 	/* should fail as boundary is not power of two */
-	name = "bounded_error_31";
+	name = "UT_TMZ_bounded_error_31";
 	if ((memzone_err = rte_memzone_reserve_bounded(name,
 			100, SOCKET_ID_ANY, 0, 32, UINT32_MAX)) != NULL) {
 		printf("%s(%s)created a memzone with invalid boundary "
@@ -734,7 +740,7 @@  test_memzone_bounded(void)
 	}
 
 	/* should fail as len is greater then boundary */
-	name = "bounded_error_32";
+	name = "UT_TMZ_bounded_error_32";
 	if ((memzone_err = rte_memzone_reserve_bounded(name,
 			100, SOCKET_ID_ANY, 0, 32, 32)) != NULL) {
 		printf("%s(%s)created a memzone with invalid boundary "
@@ -764,25 +770,25 @@  test_memzone_free(void)
 	int i;
 	char name[20];
 
-	mz[0] = rte_memzone_reserve("tempzone0", 2000, SOCKET_ID_ANY, 0);
-	mz[1] = rte_memzone_reserve("tempzone1", 4000, SOCKET_ID_ANY, 0);
+	mz[0] = rte_memzone_reserve("UT_TMZ_tempzone0", 2000, SOCKET_ID_ANY, 0);
+	mz[1] = rte_memzone_reserve("UT_TMZ_tempzone1", 4000, SOCKET_ID_ANY, 0);
 
 	if (mz[0] > mz[1])
 		return -1;
-	if (!rte_memzone_lookup("tempzone0"))
+	if (!rte_memzone_lookup("UT_TMZ_tempzone0"))
 		return -1;
-	if (!rte_memzone_lookup("tempzone1"))
+	if (!rte_memzone_lookup("UT_TMZ_tempzone1"))
 		return -1;
 
 	if (rte_memzone_free(mz[0])) {
 		printf("Fail memzone free - tempzone0\n");
 		return -1;
 	}
-	if (rte_memzone_lookup("tempzone0")) {
+	if (rte_memzone_lookup("UT_TMZ_tempzone0")) {
 		printf("Found previously free memzone - tempzone0\n");
 		return -1;
 	}
-	mz[2] = rte_memzone_reserve("tempzone2", 2000, SOCKET_ID_ANY, 0);
+	mz[2] = rte_memzone_reserve("UT_TMZ_tempzone2", 2000, SOCKET_ID_ANY, 0);
 
 	if (mz[2] > mz[1]) {
 		printf("tempzone2 should have gotten the free entry from tempzone0\n");
@@ -792,7 +798,7 @@  test_memzone_free(void)
 		printf("Fail memzone free - tempzone2\n");
 		return -1;
 	}
-	if (rte_memzone_lookup("tempzone2")) {
+	if (rte_memzone_lookup("UT_TMZ_tempzone2")) {
 		printf("Found previously free memzone - tempzone2\n");
 		return -1;
 	}
@@ -800,14 +806,14 @@  test_memzone_free(void)
 		printf("Fail memzone free - tempzone1\n");
 		return -1;
 	}
-	if (rte_memzone_lookup("tempzone1")) {
+	if (rte_memzone_lookup("UT_TMZ_tempzone1")) {
 		printf("Found previously free memzone - tempzone1\n");
 		return -1;
 	}
 
 	i = 0;
 	do {
-		snprintf(name, sizeof(name), "tempzone%u", i);
+		snprintf(name, sizeof(name), "UT_TMZ_tempzone%u", i);
 		mz[i] = rte_memzone_reserve(name, 1, SOCKET_ID_ANY, 0);
 	} while (mz[i++] != NULL);
 
@@ -815,7 +821,7 @@  test_memzone_free(void)
 		printf("Fail memzone free - tempzone0\n");
 		return -1;
 	}
-	mz[0] = rte_memzone_reserve("tempzone0new", 0, SOCKET_ID_ANY, 0);
+	mz[0] = rte_memzone_reserve("UT_TMZ_tempzone0new", 0, SOCKET_ID_ANY, 0);
 
 	if (mz[0] == NULL) {
 		printf("Fail to create memzone - tempzone0new - when MAX memzones were "
@@ -842,16 +848,16 @@  test_memzone_basic(void)
 	const struct rte_memzone *memzone4;
 	const struct rte_memzone *mz;
 
-	memzone1 = rte_memzone_reserve("testzone1", 100,
+	memzone1 = rte_memzone_reserve("UT_TMZ_testzone1", 100,
 				SOCKET_ID_ANY, 0);
 
-	memzone2 = rte_memzone_reserve("testzone2", 1000,
+	memzone2 = rte_memzone_reserve("UT_TMZ_testzone2", 1000,
 				0, 0);
 
-	memzone3 = rte_memzone_reserve("testzone3", 1000,
+	memzone3 = rte_memzone_reserve("UT_TMZ_testzone3", 1000,
 				1, 0);
 
-	memzone4 = rte_memzone_reserve("testzone4", 1024,
+	memzone4 = rte_memzone_reserve("UT_TMZ_testzone4", 1024,
 				SOCKET_ID_ANY, 0);
 
 	/* memzone3 may be NULL if we don't have NUMA */
@@ -903,12 +909,12 @@  test_memzone_basic(void)
 		return -1;
 
 	printf("test zone lookup\n");
-	mz = rte_memzone_lookup("testzone1");
+	mz = rte_memzone_lookup("UT_TMZ_testzone1");
 	if (mz != memzone1)
 		return -1;
 
 	printf("test duplcate zone name\n");
-	mz = rte_memzone_reserve("testzone1", 100,
+	mz = rte_memzone_reserve("UT_TMZ_testzone1", 100,
 			SOCKET_ID_ANY, 0);
 	if (mz != NULL)
 		return -1;
@@ -933,11 +939,12 @@  test_memzone_basic(void)
 	return 0;
 }
 
-static int memzone_walk_called;
-static void memzone_walk_clb(const struct rte_memzone *mz __rte_unused,
+static int memzone_has_dirty_remained;
+static void memzone_walk_clb(const struct rte_memzone *mz,
 			     void *arg __rte_unused)
 {
-	memzone_walk_called = 1;
+	if (strncmp(mz->name, "UT_TMZ_", strlen("UT_TMZ_")) == 0)
+		memzone_has_dirty_remained = 1;
 }
 
 static int
@@ -981,7 +988,7 @@  test_memzone(void)
 
 	printf("check memzone cleanup\n");
 	rte_memzone_walk(memzone_walk_clb, NULL);
-	if (memzone_walk_called) {
+	if (memzone_has_dirty_remained) {
 		printf("there are some memzones left after test\n");
 		rte_memzone_dump(stdout);
 		return -1;