On Fri, Apr 12, 2019 at 02:21:41PM -0400, Aaron Conole wrote: > Bruce Richardson <bruce.richard...@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 <acon...@redhat.com> > >> Reviewed-by: David Marchand <david.march...@redhat.com> > >> Acked-by: Luca Boccassi <bl...@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. >
Fair responses to all comments. Acked-by: Bruce Richardson <bruce.richard...@intel.com>
