Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores

[Bruce Richardson <bruce.richardson@intel.com>]

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.
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.

>  	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]

/Bruce