DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest introduced by tailq rework
@ 2014-11-05 12:03 Anatoly Burakov
  0 siblings, 0 replies; 4+ messages in thread
From: Anatoly Burakov @ 2014-11-05 12:03 UTC (permalink / raw)
  To: dev

As a result of moving tailq's into local memory, some tailq data
is now reserved in rte_malloc heaps (because it needs to be
shared across DPDK processes). The first thing DPDK initializes
is a log mempool, and since it creates a tailq, it reserves
space in rte_malloc heap before allocating the mempool itself.
By default, rte_malloc allocates way more space than is necessary,
so under some conditions (namely, overall memory available is low)
this results in malloc heap eating up so much memory that log
mempool is not able to allocate its memzone.

This patch fixes the unit tests to account for that change.
---
 app/test/test_eal_flags.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 21e6cca..9541619 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -52,6 +52,11 @@
 
 #include "process.h"
 
+#ifdef RTE_LIBRTE_XEN_DOM0
+#define DEFAULT_MEM_SIZE "30"
+#else
+#define DEFAULT_MEM_SIZE "8"
+#endif
 #define mp_flag "--proc-type=secondary"
 #define no_hpet "--no-hpet"
 #define no_huge "--no-huge"
@@ -616,14 +621,15 @@ test_no_huge_flag(void)
 	/* With --no-huge */
 	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2"};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2", "-m", "2"};
+	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
 	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
-			"--socket-mem=2"};
+			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
 	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
-			"-m", "2", "--socket-mem=2"};
+			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
 		return -1;
@@ -789,20 +795,20 @@ test_misc_flags(void)
 	/* With invalid --syslog */
 	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
 	/* With no-sh-conf */
-	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix };
 
 #ifdef RTE_EXEC_ENV_BSDAPP
 	return 0;
 #endif
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
@@ -923,15 +929,15 @@ test_file_prefix(void)
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-n", "2", "-m", "2",
+	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", "2",
+	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", "2",
+	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 				"--file-prefix=" memtest2 };
 
 	char prefix[32];
@@ -1025,7 +1031,6 @@ test_file_prefix(void)
 static int
 test_memory_flags(void)
 {
-	const char* mem_size = NULL;
 #ifdef RTE_EXEC_ENV_BSDAPP
 	/* BSD target doesn't support prefixes at this point */
 	const char * prefix = "";
@@ -1037,20 +1042,14 @@ test_memory_flags(void)
 	}
 	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
 #endif
-#ifdef RTE_LIBRTE_XEN_DOM0
-	mem_size = "30";
-#else
-	mem_size = "2";
-#endif
-
 
 	/* valid -m flag and mp flag */
 	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
-			"-n", "2", "-m", mem_size};
+			"-n", "2", "-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
 	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
-			"--file-prefix=" memtest, "-m", mem_size};
+			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* invalid (zero) --socket-mem flag */
 	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
@@ -1078,7 +1077,7 @@ test_memory_flags(void)
 
 	/* valid --socket-mem specified together with -m flag */
 	const char *argv8[] = {prgname, "-c", "10", "-n", "2",
-			"--file-prefix=" memtest, "-m", "2", "--socket-mem=2,2"};
+			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
 	 * extra 2 megs on socket that doesn't exist on current system */
@@ -1100,7 +1099,7 @@ test_memory_flags(void)
 
 	/* add one extra socket */
 	for (i = 0; i < num_sockets + 1; i++) {
-		snprintf(buf, sizeof(buf), "%s2", invalid_socket_mem);
+		snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE);
 		snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "%s", buf);
 
 		if (num_sockets + 1 - i > 1) {
@@ -1116,7 +1115,7 @@ test_memory_flags(void)
 
 	/* add one extra socket */
 	for (i = 0; i < num_sockets; i++) {
-		snprintf(buf, sizeof(buf), "%s2", valid_socket_mem);
+		snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE);
 		snprintf(valid_socket_mem, sizeof(valid_socket_mem), "%s", buf);
 
 		if (num_sockets - i > 1) {
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest introduced by tailq rework
  2014-11-05 13:24 ` De Lara Guarch, Pablo
@ 2014-11-05 17:48   ` Thomas Monjalon
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2014-11-05 17:48 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev

2014-11-05 13:24, De Lara Guarch, Pablo:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> > Sent: Wednesday, November 05, 2014 12:11 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest
> > introduced by tailq rework
> > 
> > As a result of moving tailq's into local memory, some tailq data
> > is now reserved in rte_malloc heaps (because it needs to be
> > shared across DPDK processes). The first thing DPDK initializes
> > is a log mempool, and since it creates a tailq, it reserves
> > space in rte_malloc heap before allocating the mempool itself.
> > By default, rte_malloc allocates way more space than is necessary,
> > so under some conditions (namely, overall memory available is low)
> > this results in malloc heap eating up so much memory that log
> > mempool is not able to allocate its memzone.
> > 
> > This patch fixes the unit tests to account for that change.
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Tested-by: Thomas Monjalon <thomas.monjalon@6wind.com>

echo 9 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
build/app/test -c 1 -n 1 -m 4

It works now. Thanks Anatoly!

Applied

Reminder for everyone: patches should be posted with --in-reply-to option
to insert the patch in the thread context of the mailing list.
It makes easier to follow discussions.
For a new version of a patchset, it's a good practice to reply to the cover
letter of the previous version.
For a bug fix (this case), it's a good idea to reply to the bug report.
So someone searching in the archives can find the fix he's looking for.

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest introduced by tailq rework
  2014-11-05 12:11 Anatoly Burakov
@ 2014-11-05 13:24 ` De Lara Guarch, Pablo
  2014-11-05 17:48   ` Thomas Monjalon
  0 siblings, 1 reply; 4+ messages in thread
From: De Lara Guarch, Pablo @ 2014-11-05 13:24 UTC (permalink / raw)
  To: Burakov, Anatoly, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Wednesday, November 05, 2014 12:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest
> introduced by tailq rework
> 
> As a result of moving tailq's into local memory, some tailq data
> is now reserved in rte_malloc heaps (because it needs to be
> shared across DPDK processes). The first thing DPDK initializes
> is a log mempool, and since it creates a tailq, it reserves
> space in rte_malloc heap before allocating the mempool itself.
> By default, rte_malloc allocates way more space than is necessary,
> so under some conditions (namely, overall memory available is low)
> this results in malloc heap eating up so much memory that log
> mempool is not able to allocate its memzone.
> 
> This patch fixes the unit tests to account for that change.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest introduced by tailq rework
@ 2014-11-05 12:11 Anatoly Burakov
  2014-11-05 13:24 ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 4+ messages in thread
From: Anatoly Burakov @ 2014-11-05 12:11 UTC (permalink / raw)
  To: dev

As a result of moving tailq's into local memory, some tailq data
is now reserved in rte_malloc heaps (because it needs to be
shared across DPDK processes). The first thing DPDK initializes
is a log mempool, and since it creates a tailq, it reserves
space in rte_malloc heap before allocating the mempool itself.
By default, rte_malloc allocates way more space than is necessary,
so under some conditions (namely, overall memory available is low)
this results in malloc heap eating up so much memory that log
mempool is not able to allocate its memzone.

This patch fixes the unit tests to account for that change.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test/test_eal_flags.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 21e6cca..9541619 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -52,6 +52,11 @@
 
 #include "process.h"
 
+#ifdef RTE_LIBRTE_XEN_DOM0
+#define DEFAULT_MEM_SIZE "30"
+#else
+#define DEFAULT_MEM_SIZE "8"
+#endif
 #define mp_flag "--proc-type=secondary"
 #define no_hpet "--no-hpet"
 #define no_huge "--no-huge"
@@ -616,14 +621,15 @@ test_no_huge_flag(void)
 	/* With --no-huge */
 	const char *argv1[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2"};
 	/* With --no-huge and -m */
-	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2", "-m", "2"};
+	const char *argv2[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
+			"-m", DEFAULT_MEM_SIZE};
 
 	/* With --no-huge and --socket-mem */
 	const char *argv3[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
-			"--socket-mem=2"};
+			"--socket-mem=" DEFAULT_MEM_SIZE};
 	/* With --no-huge, -m and --socket-mem */
 	const char *argv4[] = {prgname, prefix, no_huge, "-c", "1", "-n", "2",
-			"-m", "2", "--socket-mem=2"};
+			"-m", DEFAULT_MEM_SIZE, "--socket-mem=" DEFAULT_MEM_SIZE};
 	if (launch_proc(argv1) != 0) {
 		printf("Error - process did not run ok with --no-huge flag\n");
 		return -1;
@@ -789,20 +795,20 @@ test_misc_flags(void)
 	/* With invalid --syslog */
 	const char *argv5[] = {prgname, prefix, mp_flag, "-c", "1", "--syslog", "error"};
 	/* With no-sh-conf */
-	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv6[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix };
 
 #ifdef RTE_EXEC_ENV_BSDAPP
 	return 0;
 #endif
 	/* With --huge-dir */
-	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv7[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
 	/* With empty --huge-dir (should fail) */
-	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv8[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir"};
 	/* With invalid --huge-dir */
-	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", "2",
+	const char *argv9[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", "invalid"};
 	/* Secondary process with invalid --huge-dir (should run as flag has no
 	 * effect on secondary processes) */
@@ -923,15 +929,15 @@ test_file_prefix(void)
 #endif
 
 	/* this should fail unless the test itself is run with "memtest" prefix */
-	const char *argv0[] = {prgname, mp_flag, "-c", "1", "-n", "2", "-m", "2",
+	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", "2",
+	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", "2",
+	const char *argv2[] = {prgname, "-c", "1", "-n", "2", "-m", DEFAULT_MEM_SIZE,
 				"--file-prefix=" memtest2 };
 
 	char prefix[32];
@@ -1025,7 +1031,6 @@ test_file_prefix(void)
 static int
 test_memory_flags(void)
 {
-	const char* mem_size = NULL;
 #ifdef RTE_EXEC_ENV_BSDAPP
 	/* BSD target doesn't support prefixes at this point */
 	const char * prefix = "";
@@ -1037,20 +1042,14 @@ test_memory_flags(void)
 	}
 	snprintf(prefix, sizeof(prefix), "--file-prefix=%s", tmp);
 #endif
-#ifdef RTE_LIBRTE_XEN_DOM0
-	mem_size = "30";
-#else
-	mem_size = "2";
-#endif
-
 
 	/* valid -m flag and mp flag */
 	const char *argv0[] = {prgname, prefix, mp_flag, "-c", "10",
-			"-n", "2", "-m", mem_size};
+			"-n", "2", "-m", DEFAULT_MEM_SIZE};
 
 	/* valid -m flag */
 	const char *argv1[] = {prgname, "-c", "10", "-n", "2",
-			"--file-prefix=" memtest, "-m", mem_size};
+			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE};
 
 	/* invalid (zero) --socket-mem flag */
 	const char *argv2[] = {prgname, "-c", "10", "-n", "2",
@@ -1078,7 +1077,7 @@ test_memory_flags(void)
 
 	/* valid --socket-mem specified together with -m flag */
 	const char *argv8[] = {prgname, "-c", "10", "-n", "2",
-			"--file-prefix=" memtest, "-m", "2", "--socket-mem=2,2"};
+			"--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"};
 
 	/* construct an invalid socket mask with 2 megs on each socket plus
 	 * extra 2 megs on socket that doesn't exist on current system */
@@ -1100,7 +1099,7 @@ test_memory_flags(void)
 
 	/* add one extra socket */
 	for (i = 0; i < num_sockets + 1; i++) {
-		snprintf(buf, sizeof(buf), "%s2", invalid_socket_mem);
+		snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE);
 		snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "%s", buf);
 
 		if (num_sockets + 1 - i > 1) {
@@ -1116,7 +1115,7 @@ test_memory_flags(void)
 
 	/* add one extra socket */
 	for (i = 0; i < num_sockets; i++) {
-		snprintf(buf, sizeof(buf), "%s2", valid_socket_mem);
+		snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE);
 		snprintf(valid_socket_mem, sizeof(valid_socket_mem), "%s", buf);
 
 		if (num_sockets - i > 1) {
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-11-05 17:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-05 12:03 [dpdk-dev] [PATCH] Fix regression for eal_flags_autotest introduced by tailq rework Anatoly Burakov
2014-11-05 12:11 Anatoly Burakov
2014-11-05 13:24 ` De Lara Guarch, Pablo
2014-11-05 17:48   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).