[dpdk-dev] [PATCH] test: fix eal flags autotest hugepage file handling

[dpdk-dev] [PATCH] test: fix eal flags autotest hugepage file handling

[Anatoly Burakov]

Before 18.05, DPDK could not release memory back to the system
neither at runtime nor before shutting down. Over the course of
18.05 up to 18.11, code was introduced to release memory at
runtime, as well as an rte_eal_cleanup() function that is supposed
to release all EAL-allocated memory before shutting down DPDK.

When 3f9e31d71d63 ("test: clean up on exit") was introduced, the
test application started to use rte_eal_cleanup() to release all
used memory after execution. However, the EAL flags autotest
still relies on the old behavior of leaving stuff behind in the
hugetlbfs.

The fix is twofold. First, the test to check for leftover files
in hugetlbfs is no longer valid as it is, because test application
now removes all files from hugetlbfs after exit. However, if we
use the --legacy-mem option, then old behavior of leaving files
in hugetlbfs after execution is restored. So the first fix is to
add --legacy-mem to all the tests that expect files in hugetlbfs
to be leftover.

However, we also need to test if default memory mode *doesn't*
leave any files behind, so we also extend the test to check for
these scenarios as well. So, both memtest1 and memtest2 are run
in legacy and default mem modes, and are checked for any leftover
files that are or are not supposed to be there.

Fixes: 3f9e31d71d63 ("test: clean up on exit")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    Checkpatch will complain about string lists not being declared
    as static const char * const array - we can't do that here,
    because ``prgname`` is not a compile-time constant.

 test/test/test_eal_flags.c | 94 +++++++++++++++++++++++++++++++-------
 1 file changed, 78 insertions(+), 16 deletions(-)

diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 3a990766d..2acab9d69 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -951,10 +951,15 @@ test_file_prefix(void)
 	 * 2. try to run secondary process without a corresponding primary process
 	 * (while failing to run, it will also remove any unused hugepage files)
 	 * 3. check if current process hugefiles are still in place and are locked
-	 * 4. run a primary process with memtest1 prefix
-	 * 5. check if memtest1 hugefiles are created
-	 * 6. run a primary process with memtest2 prefix
-	 * 7. check that only memtest2 hugefiles are present in the hugedir
+	 * 4. run a primary process with memtest1 prefix in default and legacy
+	 *    mem mode
+	 * 5. check if memtest1 hugefiles are created in case of legacy mem
+	 *    mode, and deleted in case of default mem mode
+	 * 6. run a primary process with memtest2 prefix in default and legacy
+	 *    mem modes
+	 * 7. check that memtest2 hugefiles are present in the hugedir after a
+	 *    run in legacy mode, and not present at all after run in default
+	 *    mem mode
 	 */
 	char prefix[PATH_MAX] = "";
 
@@ -971,13 +976,23 @@ test_file_prefix(void)
 	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=" memtest };
 
-	/* primary process with memtest1 */
-	const char *argv1[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
-				"--file-prefix=" memtest1 };
+	/* primary process with memtest1 and default mem mode */
+	const char *argv1[] = {prgname, "-c", "1", "-n", "2", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1 };
 
-	/* primary process with memtest2 */
-	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
-				"--file-prefix=" memtest2 };
+	/* primary process with memtest1 and legacy mem mode */
+	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest1,
+			"--legacy-mem" };
+
+	/* primary process with memtest2 and legacy mem mode */
+	const char *argv3[] = {prgname, "-c", "1", "-n", "2", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2,
+			"--legacy-mem" };
+
+	/* primary process with memtest2 and default mem mode */
+	const char *argv4[] = {prgname, "-c", "1", "-n", "2", "-m",
+			DEFAULT_MEM_SIZE, "--file-prefix=" memtest2 };
 
 	/* check if files for current prefix are present */
 	if (process_hugefiles(prefix, HUGEPAGE_CHECK_EXISTS) != 1) {
@@ -1024,31 +1039,78 @@ test_file_prefix(void)
 		return -1;
 	}
 
+	/* we're running this process in default memory mode, which means it
+	 * should clean up after itself on exit and leave no hugepages behind.
+	 */
 	if (launch_proc(argv1) != 0) {
-		printf("Error - failed to run with --file-prefix=%s\n", memtest);
+		printf("Error - failed to run with --file-prefix=%s\n",
+				memtest1);
+		return -1;
+	}
+
+	/* check if memtest1_map0 is present */
+	if (process_hugefiles(memtest1, HUGEPAGE_CHECK_EXISTS) != 0) {
+		printf("Error - hugepage files for %s were not deleted!\n",
+				memtest1);
+		return -1;
+	}
+
+	/* now, we're running a process under the same prefix, but with legacy
+	 * mem mode - this should leave behind hugepage files.
+	 */
+	if (launch_proc(argv2) != 0) {
+		printf("Error - failed to run with --file-prefix=%s\n",
+				memtest1);
 		return -1;
 	}
 
 	/* check if memtest1_map0 is present */
 	if (process_hugefiles(memtest1, HUGEPAGE_CHECK_EXISTS) != 1) {
-		printf("Error - hugepage files for %s were not created!\n", memtest1);
+		printf("Error - hugepage files for %s were not created!\n",
+				memtest1);
 		return -1;
 	}
 
-	if (launch_proc(argv2) != 0) {
-		printf("Error - failed to run with --file-prefix=%s\n", memtest2);
+	if (launch_proc(argv3) != 0) {
+		printf("Error - failed to run with --file-prefix=%s\n",
+				memtest2);
 		return -1;
 	}
 
 	/* check if hugefiles for memtest2 are present */
 	if (process_hugefiles(memtest2, HUGEPAGE_CHECK_EXISTS) != 1) {
-		printf("Error - hugepage files for %s were not created!\n", memtest2);
+		printf("Error - hugepage files for %s were not created!\n",
+				memtest2);
 		return -1;
 	}
 
 	/* check if hugefiles for memtest1 are present */
 	if (process_hugefiles(memtest1, HUGEPAGE_CHECK_EXISTS) != 0) {
-		printf("Error - hugepage files for %s were not deleted!\n", memtest1);
+		printf("Error - hugepage files for %s were not deleted!\n",
+				memtest1);
+		return -1;
+	}
+
+	/* this process will run in default mem mode, so it should not leave any
+	 * hugepage files behind.
+	 */
+	if (launch_proc(argv4) != 0) {
+		printf("Error - failed to run with --file-prefix=%s\n",
+				memtest2);
+		return -1;
+	}
+
+	/* check if hugefiles for memtest2 are present */
+	if (process_hugefiles(memtest2, HUGEPAGE_CHECK_EXISTS) != 0) {
+		printf("Error - hugepage files for %s were not deleted!\n",
+				memtest2);
+		return -1;
+	}
+
+	/* check if hugefiles for memtest1 are present */
+	if (process_hugefiles(memtest1, HUGEPAGE_CHECK_EXISTS) != 0) {
+		printf("Error - hugepage files for %s were not deleted!\n",
+				memtest1);
 		return -1;
 	}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH] test: fix eal flags autotest hugepage file handling

[Thomas Monjalon]

15/11/2018 13:18, Anatoly Burakov:
> Before 18.05, DPDK could not release memory back to the system
> neither at runtime nor before shutting down. Over the course of
> 18.05 up to 18.11, code was introduced to release memory at
> runtime, as well as an rte_eal_cleanup() function that is supposed
> to release all EAL-allocated memory before shutting down DPDK.
> 
> When 3f9e31d71d63 ("test: clean up on exit") was introduced, the
> test application started to use rte_eal_cleanup() to release all
> used memory after execution. However, the EAL flags autotest
> still relies on the old behavior of leaving stuff behind in the
> hugetlbfs.
> 
> The fix is twofold. First, the test to check for leftover files
> in hugetlbfs is no longer valid as it is, because test application
> now removes all files from hugetlbfs after exit. However, if we
> use the --legacy-mem option, then old behavior of leaving files
> in hugetlbfs after execution is restored. So the first fix is to
> add --legacy-mem to all the tests that expect files in hugetlbfs
> to be leftover.
> 
> However, we also need to test if default memory mode *doesn't*
> leave any files behind, so we also extend the test to check for
> these scenarios as well. So, both memtest1 and memtest2 are run
> in legacy and default mem modes, and are checked for any leftover
> files that are or are not supposed to be there.
> 
> Fixes: 3f9e31d71d63 ("test: clean up on exit")
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

I guess you know what you are doing :-)

Applied, thanks