Re: Issue with patch "app/testpmd: detect numa socket count"

Re: Issue with patch "app/testpmd: detect numa socket count"

[Qiu, Michael]

Sorry, I forgot to do that, add CC to mailing list 

Thanks,
Michael

> 在 2015年12月19日，上午7:32，Stephen Hurd <shurd@broadcom.com> 写道：
> 
> Thanks, I can reproduce it now.
> 
> The intent was to set max_socket when detecting lcores, not when selecting active ones, so there's a bug in my patch.  I'll actually be working on DPDK next week, so I'll have fix on Monday or Tuesday.
> 
> As for parsing /sys/, that's not likely to work on FreeBSD, and since we already detect all lcores and their socket, it makes sense to set max_socket there.
> 
> My request for the additional logging was so I could see the lines that correspond to these:
> 
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Detected lcore 1 as core 0 on socket 1
> EAL: Detected lcore 2 as core 1 on socket 0
> EAL: Detected lcore 3 as core 1 on socket 1
> 
> Which would indicate that EAL was able to detect a higher max socket even though it's not enabled.
> 
> Is there a reason we're not CCing the mailing list?
> 
> 
> -- Stephen Hurd
> 
> 
> -----Original Message-----
> From: Qiu, Michael [mailto:michael.qiu@intel.com] 
> Sent: Thursday, December 17, 2015 10:31 PM
> To: Stephen Hurd; De Lara Guarch, Pablo
> Cc: Tan, Jianfeng; Gonzalez Monroy, Sergio
> Subject: Re: Issue with patch "app/testpmd: detect numa socket count"
> 
> That's the bug, I have two socket in my system, also from log you could see:
> 
> EAL: Requesting 512 pages of size 2MB from socket 1
> 
> command is : ./testpmd -c 0x3 -n 4 -- -i -socket-num=1
> 
> the root cause is that you only check the enabled lcore which is on
> socket 0, and then mark max socket as 1,  also your could has issue when
> in --numa
> 
> Because when numa enabled, still we run with coremask 0x03, the max
> socket will never be 2, which will lead lots of issues.
> 
> Thanks,
> Michael
> 
> 
>> On 2015/12/18 9:57, Stephen Hurd wrote:
>> Can you provide more log info (the socket to core mapping especially) and the entire command line (or at least the entire EAL set)?  The error suggests that you only have one numa socket (socket 0) on that system and are attempting to allocate memory on socket 1.
>> 
>> My patch redefined the max socket to be the last socket the system has whereas the old code allowed specifying sockets that aren't physically present in the system.
>> 
>> 
>> -- Stephen Hurd
>> 
>> 
>> -----Original Message-----
>> From: Qiu, Michael [mailto:michael.qiu@intel.com] 
>> Sent: Thursday, December 17, 2015 5:42 PM
>> To: Stephen Hurd; De Lara Guarch, Pablo
>> Subject: Issue with patch "app/testpmd: detect numa socket count"
>> 
>> Hi, Stephen
>> 
>> I just see this patch and found some issue with it.
>> 
>> When I start testpmd with -c 0x3 but with socket-num 1, that means run
>> lcore in socket 0, but want hugepage allocated in socket 1, in previous,
>> it works, I don't know why you force it in the socket lcore locates.I
>> would like to give a warning instead of failure. just like before:
>> 
>> EAL: Requesting 512 pages of size 2MB from socket 1
>> EAL: TSC frequency is ~2294689 KHz
>> EAL: WARNING: Master core has no memory on local socket!
>> 
>> After your patch:
>> 
>> EAL: No probed ethernet devices
>> Interactive-mode selected
>> EAL: Error - exiting with code: 1
>>  Cause: The socket number should be < 1
>> 
>> 
>> Thanks,
>> Michael
> 
> 

[PATCH] app/testpmd Fix max_socket detection

[Stephen Hurd]

Previously, max_socket was set to the highest numbered socket with
an enabled lcore.  The intent is to set it to the highest socket
regardless of it being enabled.

Change-Id: I6306af0f90aa3c1fc5ffed75d1eed8297d29e132
Signed-off-by: Stephen Hurd <shurd@broadcom.com>
---
 app/test-pmd/testpmd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6129c26..26a2cce 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -359,17 +359,17 @@ set_default_fwd_lcores_config(void)
 
 	nb_lc = 0;
 	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		if (sock_num > max_socket) {
+			if (sock_num > RTE_MAX_NUMA_NODES)
+				rte_exit(EXIT_FAILURE, "Total sockets greater than %u\n", RTE_MAX_NUMA_NODES);
+			max_socket = sock_num;
+		}
 		if (! rte_lcore_is_enabled(i))
 			continue;
 		if (i == rte_get_master_lcore())
 			continue;
 		fwd_lcores_cpuids[nb_lc++] = i;
 		sock_num = rte_lcore_to_socket_id(i) + 1;
-		if (sock_num > max_socket) {
-			if (sock_num > RTE_MAX_NUMA_NODES)
-				rte_exit(EXIT_FAILURE, "Total sockets greater than %u\n", RTE_MAX_NUMA_NODES);
-			max_socket = sock_num;
-		}
 	}
 	nb_lcores = (lcoreid_t) nb_lc;
 	nb_cfg_lcores = nb_lcores;
-- 
1.9.1

[PATCH v2] app/testpmd Fix max_socket detection

[Stephen Hurd]

Previously, max_socket was set to the highest numbered socket with
an enabled lcore.  The intent is to set it to the highest socket
regardless of it being enabled.

Change-Id: I6306af0f90aa3c1fc5ffed75d1eed8297d29e132
Signed-off-by: Stephen Hurd <shurd@broadcom.com>

v2: Forgot to commit before sending email... sorry for the nouse.
---
 app/test-pmd/testpmd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6129c26..a4088f9 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -359,17 +359,17 @@ set_default_fwd_lcores_config(void)
 
 	nb_lc = 0;
 	for (i = 0; i < RTE_MAX_LCORE; i++) {
-		if (! rte_lcore_is_enabled(i))
-			continue;
-		if (i == rte_get_master_lcore())
-			continue;
-		fwd_lcores_cpuids[nb_lc++] = i;
 		sock_num = rte_lcore_to_socket_id(i) + 1;
 		if (sock_num > max_socket) {
 			if (sock_num > RTE_MAX_NUMA_NODES)
 				rte_exit(EXIT_FAILURE, "Total sockets greater than %u\n", RTE_MAX_NUMA_NODES);
 			max_socket = sock_num;
 		}
+		if (! rte_lcore_is_enabled(i))
+			continue;
+		if (i == rte_get_master_lcore())
+			continue;
+		fwd_lcores_cpuids[nb_lc++] = i;
 	}
 	nb_lcores = (lcoreid_t) nb_lc;
 	nb_cfg_lcores = nb_lcores;
-- 
1.9.1

Re: [PATCH v2] app/testpmd Fix max_socket detection

[Bruce Richardson]

On Wed, Jan 13, 2016 at 02:23:36PM -0800, Stephen Hurd wrote:
> Previously, max_socket was set to the highest numbered socket with
> an enabled lcore.  The intent is to set it to the highest socket
> regardless of it being enabled.
> 

Can you clarify why this changes is necessary? Is it causing a bug somewhere?

thanks,
/Bruce

Re: [PATCH v2] app/testpmd Fix max_socket detection

[Stephen Hurd]

Yes, this is cause a bug which prevents cross-socket testing... per michael.qiu@intel.com on Fri 12/18/2015 5:33 PM  (unable to find email on the list)

>> Hi, Stephen
>> 
>> I just see this patch and found some issue with it.
>> 
>> When I start testpmd with -c 0x3 but with socket-num 1, that means run
>> lcore in socket 0, but want hugepage allocated in socket 1, in previous,
>> it works, I don't know why you force it in the socket lcore locates.I
>> would like to give a warning instead of failure. just like before:
>> 
>> EAL: Requesting 512 pages of size 2MB from socket 1
>> EAL: TSC frequency is ~2294689 KHz
>> EAL: WARNING: Master core has no memory on local socket!
>> 
>> After your patch:
>> 
>> EAL: No probed ethernet devices
>> Interactive-mode selected
>> EAL: Error - exiting with code: 1
>>  Cause: The socket number should be < 1
>>

His example command was:
./testpmd -c 0x3 -n 4 -- -i -socket-num=1


-- Stephen Hurd


-----Original Message-----
From: Bruce Richardson [mailto:bruce.richardson@intel.com] 
Sent: Thursday, January 14, 2016 5:44 AM
To: Stephen Hurd
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd Fix max_socket detection

On Wed, Jan 13, 2016 at 02:23:36PM -0800, Stephen Hurd wrote:
> Previously, max_socket was set to the highest numbered socket with
> an enabled lcore.  The intent is to set it to the highest socket
> regardless of it being enabled.
> 

Can you clarify why this changes is necessary? Is it causing a bug somewhere?

thanks,
/Bruce

Re: [PATCH v2] app/testpmd Fix max_socket detection

[Stephen Hurd]

Found it...
http://www.dpdk.org/ml/archives/dev/2015-December/030564.html


-- Stephen Hurd


-----Original Message-----
From: Bruce Richardson [mailto:bruce.richardson@intel.com] 
Sent: Thursday, January 14, 2016 5:44 AM
To: Stephen Hurd
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd Fix max_socket detection

On Wed, Jan 13, 2016 at 02:23:36PM -0800, Stephen Hurd wrote:
> Previously, max_socket was set to the highest numbered socket with
> an enabled lcore.  The intent is to set it to the highest socket
> regardless of it being enabled.
> 

Can you clarify why this changes is necessary? Is it causing a bug somewhere?

thanks,
/Bruce

Re: [PATCH v2] app/testpmd Fix max_socket detection

[Bruce Richardson]

On Fri, Jan 15, 2016 at 01:01:04AM +0000, Stephen Hurd wrote:
> Found it...
> http://www.dpdk.org/ml/archives/dev/2015-December/030564.html
> 
> 
> -- Stephen Hurd
> 

Thanks for that. For some reason I missed the rest of the email thread from earlier.

/Bruce

> 
> -----Original Message-----
> From: Bruce Richardson [mailto:bruce.richardson@intel.com] 
> Sent: Thursday, January 14, 2016 5:44 AM
> To: Stephen Hurd
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd Fix max_socket detection
> 
> On Wed, Jan 13, 2016 at 02:23:36PM -0800, Stephen Hurd wrote:
> > Previously, max_socket was set to the highest numbered socket with
> > an enabled lcore.  The intent is to set it to the highest socket
> > regardless of it being enabled.
> > 
> 
> Can you clarify why this changes is necessary? Is it causing a bug somewhere?
> 
> thanks,
> /Bruce
> 
> 

Re: [PATCH v2] app/testpmd Fix max_socket detection

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hurd
> Sent: Wednesday, January 13, 2016 10:24 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] app/testpmd Fix max_socket detection
> 
> Previously, max_socket was set to the highest numbered socket with
> an enabled lcore.  The intent is to set it to the highest socket
> regardless of it being enabled.
> 
> Change-Id: I6306af0f90aa3c1fc5ffed75d1eed8297d29e132
> Signed-off-by: Stephen Hurd <shurd@broadcom.com>
> 
> v2: Forgot to commit before sending email... sorry for the nouse.
> ---
>  app/test-pmd/testpmd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 6129c26..a4088f9 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -359,17 +359,17 @@ set_default_fwd_lcores_config(void)
> 
>  	nb_lc = 0;
>  	for (i = 0; i < RTE_MAX_LCORE; i++) {
> -		if (! rte_lcore_is_enabled(i))
> -			continue;
> -		if (i == rte_get_master_lcore())
> -			continue;
> -		fwd_lcores_cpuids[nb_lc++] = i;
>  		sock_num = rte_lcore_to_socket_id(i) + 1;
>  		if (sock_num > max_socket) {
>  			if (sock_num > RTE_MAX_NUMA_NODES)
>  				rte_exit(EXIT_FAILURE, "Total sockets greater
> than %u\n", RTE_MAX_NUMA_NODES);
>  			max_socket = sock_num;
>  		}
> +		if (! rte_lcore_is_enabled(i))
> +			continue;
> +		if (i == rte_get_master_lcore())
> +			continue;
> +		fwd_lcores_cpuids[nb_lc++] = i;
>  	}
>  	nb_lcores = (lcoreid_t) nb_lc;
>  	nb_cfg_lcores = nb_lcores;
> --
> 1.9.1

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

Re: [PATCH v2] app/testpmd Fix max_socket detection

[Thomas Monjalon]

> > Previously, max_socket was set to the highest numbered socket with
> > an enabled lcore.  The intent is to set it to the highest socket
> > regardless of it being enabled.
> > 
> > Change-Id: I6306af0f90aa3c1fc5ffed75d1eed8297d29e132
> > Signed-off-by: Stephen Hurd <shurd@broadcom.com>
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Fixes: 7acf894d07d1 ("app/testpmd: detect numa socket count")

Applied, thanks