From: Bruce Richardson <bruce.richardson@intel.com>
To: Aaron Conole <aconole@redhat.com>
Cc: 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>
Subject: Re: [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores
Date: Mon, 15 Apr 2019 10:14:30 +0100 [thread overview]
Message-ID: <20190415091430.GB1846@bricha3-MOBL.ger.corp.intel.com> (raw)
Message-ID: <20190415091430.QtzPKylb4OR9LBFsS9_MHQ4yls8heFdezcs7vB2nIks@z> (raw)
In-Reply-To: <f7ty34f6q4q.fsf@dhcp-25.97.bos.redhat.com>
On Fri, Apr 12, 2019 at 02:21:41PM -0400, Aaron Conole wrote:
> 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.
>
Fair responses to all comments.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
next prev parent reply other threads:[~2019-04-15 9:14 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-11 19:52 [dpdk-dev] [PATCH 0/3] travis: enhancements for build (plus a meson fix) Aaron Conole
2019-04-11 19:52 ` Aaron Conole
2019-04-11 19:52 ` [dpdk-dev] [PATCH 1/3] travis: enable ccache Aaron Conole
2019-04-11 19:52 ` Aaron Conole
2019-04-12 7:46 ` David Marchand
2019-04-12 7:46 ` David Marchand
2019-04-12 9:16 ` Luca Boccassi
2019-04-12 9:16 ` Luca Boccassi
2019-04-11 19:52 ` [dpdk-dev] [PATCH 2/3] travis: add a distinguisher to the 'extra' builds Aaron Conole
2019-04-11 19:52 ` Aaron Conole
2019-04-12 7:46 ` David Marchand
2019-04-12 7:46 ` David Marchand
2019-04-12 9:17 ` Luca Boccassi
2019-04-12 9:17 ` Luca Boccassi
2019-04-11 19:52 ` [dpdk-dev] [PATCH 3/3] app/test/meson: auto detect number of cores Aaron Conole
2019-04-11 19:52 ` Aaron Conole
2019-04-12 7:46 ` David Marchand
2019-04-12 7:46 ` David Marchand
2019-04-12 9:06 ` Bruce Richardson
2019-04-12 9:06 ` Bruce Richardson
2019-04-12 9:17 ` Luca Boccassi
2019-04-12 9:17 ` Luca Boccassi
2019-04-12 16:21 ` [dpdk-dev] [PATCH v2 0/3] travis: enhancements for build (plus a meson fix) Aaron Conole
2019-04-12 16:21 ` Aaron Conole
2019-04-12 16:21 ` [dpdk-dev] [PATCH v2 1/3] travis: enable ccache Aaron Conole
2019-04-12 16:21 ` Aaron Conole
2019-04-12 16:21 ` [dpdk-dev] [PATCH v2 2/3] travis: add a distinguisher to the 'extra' builds Aaron Conole
2019-04-12 16:21 ` Aaron Conole
2019-04-12 16:21 ` [dpdk-dev] [PATCH v2 3/3] app/test/meson: auto detect number of cores Aaron Conole
2019-04-12 16:21 ` Aaron Conole
2019-04-12 16:36 ` Bruce Richardson
2019-04-12 16:36 ` Bruce Richardson
2019-04-12 18:21 ` Aaron Conole
2019-04-12 18:21 ` Aaron Conole
2019-04-15 9:14 ` Bruce Richardson [this message]
2019-04-15 9:14 ` Bruce Richardson
2019-04-17 12:09 ` [dpdk-dev] [PATCH v2 0/3] travis: enhancements for build (plus a meson fix) Thomas Monjalon
2019-04-17 12:09 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190415091430.GB1846@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=aconole@redhat.com \
--cc=agalyax.babu.radhakrishnan@intel.com \
--cc=bluca@debian.org \
--cc=dev@dpdk.org \
--cc=dmarchan@redhat.com \
--cc=reshma.pattan@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).