From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <aconole@redhat.com>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id 123477CCA
 for <dev@dpdk.org>; Fri, 12 Apr 2019 20:21:45 +0200 (CEST)
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
 [10.5.11.12])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id 6961B309EAB7;
 Fri, 12 Apr 2019 18:21:44 +0000 (UTC)
Received: from dhcp-25.97.bos.redhat.com (ovpn-122-94.rdu2.redhat.com
 [10.10.122.94])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id C292E60FAB;
 Fri, 12 Apr 2019 18:21:41 +0000 (UTC)
From: Aaron Conole <aconole@redhat.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: Aaron Conole <aconole@redhat.com>, dev@dpdk.org,
 Luca Boccassi <bluca@debian.org>, Reshma Pattan <reshma.pattan@intel.com>,
 Agalya Babu RadhaKrishnan <agalyax.babu.radhakrishnan@intel.com>,
 David Marchand <dmarchan@redhat.com>
References: <20190411195229.7841-1-aconole@redhat.com>
 <20190412162141.23327-1-aconole@redhat.com>
 <20190412162141.23327-4-aconole@redhat.com>
 <20190412163628.GA1842@bricha3-MOBL.ger.corp.intel.com>
Date: Fri, 12 Apr 2019 14:21:41 -0400
In-Reply-To: <20190412163628.GA1842@bricha3-MOBL.ger.corp.intel.com> (Bruce
 Richardson's message of "Fri, 12 Apr 2019 17:36:28 +0100")
Message-ID: <f7ty34f6q4q.fsf@dhcp-25.97.bos.redhat.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.5.110.49]); Fri, 12 Apr 2019 18:21:44 +0000 (UTC)
Subject: Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of
	cores
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 12 Apr 2019 18:21:45 -0000

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Apr 12, 2019 at 12:21:41PM -0400, Aaron Conole wrote:
>> The arguments being passed will cause failures on laptops that have,
>> for instance, 2 cores only.  Most of the tests don't require more
>> than a single core.  Some require multiple cores (but those tests
>> should be modified to 'SKIP' when the correct number of cores
>> aren't available).
>> 
>> The unit test results shouldn't be impacted by this change, but it
>> allows for a future enhancement to pass flags such as '--no-huge'.
>> 
>> Also include a fix to a reported issue with running on FreeBSD.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> Reviewed-by: David Marchand <david.marchand@redhat.com>
>> Acked-by: Luca Boccassi <bluca@debian.org>
>> ---
>> v2:
>> * Fix a spelling mistake
>> * Add support for FreeBSD
>> * Include a default fallback
>> * Use a more robust core-mask argument source (rather than lscpu)
>> 
>> Conflicts with http://patches.dpdk.org/patch/50850/
>> 
>>  app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)
>> 
>> diff --git a/app/test/meson.build b/app/test/meson.build
>> index 867cc5863..5e056eb59 100644
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> @@ -344,17 +344,43 @@ if get_option('tests')
>>  	timeout_seconds = 600
>>  	timeout_seconds_fast = 10
>>  
>> +	# Retrieve the number of CPU cores, defaulting to 4.
>> +	num_cores = '0-3'
>> +	if host_machine.system() == 'linux'
>> +		num_cores = run_command('cat',
>> +					'/sys/devices/system/cpu/present'
>> +				       ).stdout().strip()
>> +	elif host_machine.system() == 'freebsd'
>> +		snum_cores = run_command('/sbin/sysctl', '-n',
>> +					 'hw.ncpu').stdout().strip()
>> +		inum_cores = snum_cores.to_int() - 1
>> +                num_cores = '0-@0@'.format(inum_cores)
>> +	endif
>> +
>> +	num_cores_arg = '-l ' + num_cores
>> +
>> +	test_args = [num_cores_arg, '-n 4']
>
> This -n 4 parameter can be dropped. Four is the default setting IIRC.

For another patch.  I thought about doing it with this one, but I'd
rather keep the changes a little bit traceable.  If you think I should
resubmit with it dropped, I will.

> I also wonder are the parameters coming through to the app correctly,
> generally meson does not work well with parameters with spaces in them -
> I'd expect the "-l" and the num_cores values to be separated in the array.
> I also think num_cores_arg value could be dropped too.
>
> If it works though, I'm ok to keep as-is though.

It does work.  Actually, I needed to change because on some of the VM
setups (including the one used by Travis) the '-c f' arg errors because
it wants 4 cores, and only 2 exist.  Either way, this patch doesn't
change the spacing being passed with args :)

>>  	foreach arg : fast_parallel_test_names
>> -		test(arg, dpdk_test,
>> -			env : ['DPDK_TEST=' + arg],
>> -			args : ['-c f','-n 4', '--file-prefix=@0@'.format(arg)],
>> +		if host_machine.system() == 'linux'
>> +			test(arg, dpdk_test,
>> +				  env : ['DPDK_TEST=' + arg],
>> +				  args : test_args +
>> +					 ['--file-prefix=@0@'.format(arg)],
>> +			timeout : timeout_seconds_fast,
>> +			suite : 'fast-tests')
>> +		else
>> +			test(arg, dpdk_test,
>> +				env : ['DPDK_TEST=' + arg],
>> +				args : test_args,
>>  			timeout : timeout_seconds_fast,
>>  			suite : 'fast-tests')
>> +		endif
>>  	endforeach
>
> While this is needed now, I think in the medium term we should have the
> "file-prefix" flag being a warning rather than a hard-error on FreeBSD.
> [i.e. keep this, but we should fix it in 19.08 to be shorter]

Agreed.  Probably a good cleanup in the future.

> /Bruce

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id D1D35A0096
	for <public@inbox.dpdk.org>; Fri, 12 Apr 2019 20:21:48 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 65A211B10D;
	Fri, 12 Apr 2019 20:21:46 +0200 (CEST)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id 123477CCA
 for <dev@dpdk.org>; Fri, 12 Apr 2019 20:21:45 +0200 (CEST)
Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com
 [10.5.11.12])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1.redhat.com (Postfix) with ESMTPS id 6961B309EAB7;
 Fri, 12 Apr 2019 18:21:44 +0000 (UTC)
Received: from dhcp-25.97.bos.redhat.com (ovpn-122-94.rdu2.redhat.com
 [10.10.122.94])
 by smtp.corp.redhat.com (Postfix) with ESMTPS id C292E60FAB;
 Fri, 12 Apr 2019 18:21:41 +0000 (UTC)
From: Aaron Conole <aconole@redhat.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: Aaron Conole <aconole@redhat.com>, dev@dpdk.org,
 Luca Boccassi <bluca@debian.org>, Reshma Pattan <reshma.pattan@intel.com>,
 Agalya Babu RadhaKrishnan <agalyax.babu.radhakrishnan@intel.com>,
 David Marchand <dmarchan@redhat.com>
References: <20190411195229.7841-1-aconole@redhat.com>
 <20190412162141.23327-1-aconole@redhat.com>
 <20190412162141.23327-4-aconole@redhat.com>
 <20190412163628.GA1842@bricha3-MOBL.ger.corp.intel.com>
Date: Fri, 12 Apr 2019 14:21:41 -0400
In-Reply-To: <20190412163628.GA1842@bricha3-MOBL.ger.corp.intel.com> (Bruce
 Richardson's message of "Fri, 12 Apr 2019 17:36:28 +0100")
Message-ID: <f7ty34f6q4q.fsf@dhcp-25.97.bos.redhat.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
 (mx1.redhat.com [10.5.110.49]); Fri, 12 Apr 2019 18:21:44 +0000 (UTC)
Subject: Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of
	cores
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190412182141.7LGILi1UOfhMED9ZNRdubVL5w9Wn1YnbTloeTTsyLzg@z>

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Apr 12, 2019 at 12:21:41PM -0400, Aaron Conole wrote:
>> The arguments being passed will cause failures on laptops that have,
>> for instance, 2 cores only.  Most of the tests don't require more
>> than a single core.  Some require multiple cores (but those tests
>> should be modified to 'SKIP' when the correct number of cores
>> aren't available).
>> 
>> The unit test results shouldn't be impacted by this change, but it
>> allows for a future enhancement to pass flags such as '--no-huge'.
>> 
>> Also include a fix to a reported issue with running on FreeBSD.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> Reviewed-by: David Marchand <david.marchand@redhat.com>
>> Acked-by: Luca Boccassi <bluca@debian.org>
>> ---
>> v2:
>> * Fix a spelling mistake
>> * Add support for FreeBSD
>> * Include a default fallback
>> * Use a more robust core-mask argument source (rather than lscpu)
>> 
>> Conflicts with http://patches.dpdk.org/patch/50850/
>> 
>>  app/test/meson.build | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)
>> 
>> diff --git a/app/test/meson.build b/app/test/meson.build
>> index 867cc5863..5e056eb59 100644
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> @@ -344,17 +344,43 @@ if get_option('tests')
>>  	timeout_seconds = 600
>>  	timeout_seconds_fast = 10
>>  
>> +	# Retrieve the number of CPU cores, defaulting to 4.
>> +	num_cores = '0-3'
>> +	if host_machine.system() == 'linux'
>> +		num_cores = run_command('cat',
>> +					'/sys/devices/system/cpu/present'
>> +				       ).stdout().strip()
>> +	elif host_machine.system() == 'freebsd'
>> +		snum_cores = run_command('/sbin/sysctl', '-n',
>> +					 'hw.ncpu').stdout().strip()
>> +		inum_cores = snum_cores.to_int() - 1
>> +                num_cores = '0-@0@'.format(inum_cores)
>> +	endif
>> +
>> +	num_cores_arg = '-l ' + num_cores
>> +
>> +	test_args = [num_cores_arg, '-n 4']
>
> This -n 4 parameter can be dropped. Four is the default setting IIRC.

For another patch.  I thought about doing it with this one, but I'd
rather keep the changes a little bit traceable.  If you think I should
resubmit with it dropped, I will.

> I also wonder are the parameters coming through to the app correctly,
> generally meson does not work well with parameters with spaces in them -
> I'd expect the "-l" and the num_cores values to be separated in the array.
> I also think num_cores_arg value could be dropped too.
>
> If it works though, I'm ok to keep as-is though.

It does work.  Actually, I needed to change because on some of the VM
setups (including the one used by Travis) the '-c f' arg errors because
it wants 4 cores, and only 2 exist.  Either way, this patch doesn't
change the spacing being passed with args :)

>>  	foreach arg : fast_parallel_test_names
>> -		test(arg, dpdk_test,
>> -			env : ['DPDK_TEST=' + arg],
>> -			args : ['-c f','-n 4', '--file-prefix=@0@'.format(arg)],
>> +		if host_machine.system() == 'linux'
>> +			test(arg, dpdk_test,
>> +				  env : ['DPDK_TEST=' + arg],
>> +				  args : test_args +
>> +					 ['--file-prefix=@0@'.format(arg)],
>> +			timeout : timeout_seconds_fast,
>> +			suite : 'fast-tests')
>> +		else
>> +			test(arg, dpdk_test,
>> +				env : ['DPDK_TEST=' + arg],
>> +				args : test_args,
>>  			timeout : timeout_seconds_fast,
>>  			suite : 'fast-tests')
>> +		endif
>>  	endforeach
>
> While this is needed now, I think in the medium term we should have the
> "file-prefix" flag being a warning rather than a hard-error on FreeBSD.
> [i.e. keep this, but we should fix it in 19.08 to be shorter]

Agreed.  Probably a good cleanup in the future.

> /Bruce