From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 732F21B131 for ; Mon, 15 Apr 2019 11:14:34 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Apr 2019 02:14:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,353,1549958400"; d="scan'208";a="149507851" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.220.103]) by FMSMGA003.fm.intel.com with SMTP; 15 Apr 2019 02:14:31 -0700 Received: by (sSMTP sendmail emulation); Mon, 15 Apr 2019 10:14:30 +0100 Date: Mon, 15 Apr 2019 10:14:30 +0100 From: Bruce Richardson To: Aaron Conole Cc: dev@dpdk.org, Luca Boccassi , Reshma Pattan , Agalya Babu RadhaKrishnan , David Marchand Message-ID: <20190415091430.GB1846@bricha3-MOBL.ger.corp.intel.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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Apr 2019 09:14:35 -0000 On Fri, Apr 12, 2019 at 02:21:41PM -0400, Aaron Conole wrote: > Bruce Richardson 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 > >> Reviewed-by: David Marchand > >> Acked-by: Luca Boccassi > >> --- > >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 7DA67A00E6 for ; Mon, 15 Apr 2019 11:14:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9C34D1B128; Mon, 15 Apr 2019 11:14:35 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 732F21B131 for ; Mon, 15 Apr 2019 11:14:34 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Apr 2019 02:14:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,353,1549958400"; d="scan'208";a="149507851" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.220.103]) by FMSMGA003.fm.intel.com with SMTP; 15 Apr 2019 02:14:31 -0700 Received: by (sSMTP sendmail emulation); Mon, 15 Apr 2019 10:14:30 +0100 Date: Mon, 15 Apr 2019 10:14:30 +0100 From: Bruce Richardson To: Aaron Conole Cc: dev@dpdk.org, Luca Boccassi , Reshma Pattan , Agalya Babu RadhaKrishnan , David Marchand Message-ID: <20190415091430.GB1846@bricha3-MOBL.ger.corp.intel.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> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190415091430.QtzPKylb4OR9LBFsS9_MHQ4yls8heFdezcs7vB2nIks@z> On Fri, Apr 12, 2019 at 02:21:41PM -0400, Aaron Conole wrote: > Bruce Richardson 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 > >> Reviewed-by: David Marchand > >> Acked-by: Luca Boccassi > >> --- > >> 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