From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 58657A056A; Fri, 6 Mar 2020 17:33:43 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B243F1BFCE; Fri, 6 Mar 2020 17:33:42 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 8850A2BA8 for ; Fri, 6 Mar 2020 17:33:40 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Mar 2020 08:33:39 -0800 X-IronPort-AV: E=Sophos;i="5.70,522,1574150400"; d="scan'208";a="234852221" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.14.180]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 06 Mar 2020 08:33:36 -0800 Date: Fri, 6 Mar 2020 16:33:33 +0000 From: Bruce Richardson To: Aaron Conole Cc: David Marchand , Ruifeng Wang , "maicolgabriel@hotmail.com" , "dev@dpdk.org" , Gavin Hu , Honnappa Nagarahalli , "juraj.linkes@pantheon.tech" , nd Message-ID: <20200306163333.GA311@bricha3-MOBL.ger.corp.intel.com> References: <20200225073236.135581-1-ruifeng.wang@arm.com> <20200228041904.195597-1-ruifeng.wang@arm.com> <20200228041904.195597-3-ruifeng.wang@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.1 (2019-06-15) Subject: Re: [dpdk-dev] [PATCH v2 2/2] ci: add test suite run without hugepage 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" On Fri, Mar 06, 2020 at 11:16:24AM -0500, Aaron Conole wrote: > David Marchand writes: > > > On Fri, Mar 6, 2020 at 4:57 PM Aaron Conole wrote: > >> > >> Ruifeng Wang writes: > >> > >> >> -----Original Message----- > >> >> From: Aaron Conole > >> >> Sent: Thursday, March 5, 2020 22:37 > >> >> To: Ruifeng Wang > >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org; > >> >> david.marchand@redhat.com; Gavin Hu ; Honnappa > >> >> Nagarahalli ; juraj.linkes@pantheon.tech; > >> >> nd > >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage > >> >> > >> >> Ruifeng Wang writes: > >> >> > >> >> >> -----Original Message----- > >> >> >> From: Aaron Conole > >> >> >> Sent: Thursday, March 5, 2020 01:31 > >> >> >> To: Ruifeng Wang > >> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; > >> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu > >> >> ; > >> >> >> Honnappa Nagarahalli ; > >> >> >> juraj.linkes@pantheon.tech; nd > >> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage > >> >> >> > >> >> >> Ruifeng Wang writes: > >> >> >> > >> >> >> > This test suite is derived from fast-tests suite. Cases in this > >> >> >> > suite are run with '--no-huge' flag. > >> >> >> > > >> >> >> > The suite aims to cover as many as possible test cases out of the > >> >> >> > fast-tests suites in the environments without huge pages support, > >> >> >> > like containers. > >> >> >> > > >> >> >> > Signed-off-by: Ruifeng Wang > >> >> >> > Reviewed-by: Gavin Hu > >> >> >> > --- > >> >> >> > >> >> >> I like this much more. Few comments. > >> >> >> > >> >> >> > .travis.yml | 10 +++++-- > >> >> >> > app/test/meson.build | 71 > >> >> >> > ++++++++++++++++++++++++++++++++++++++++++++ > >> >> >> > >> >> >> You should update doc/guides/prog_guide/meson_ut.rst to include some > >> >> >> detail about the new tests suite. > >> >> >> > >> >> > Thanks. Will update document in next version. > >> >> > > >> >> >> > 2 files changed, 79 insertions(+), 2 deletions(-) > >> >> >> > > >> >> >> > diff --git a/.travis.yml b/.travis.yml index b64a81bd0..eed1d96db > >> >> >> > 100644 > >> >> >> > --- a/.travis.yml > >> >> >> > +++ b/.travis.yml > >> >> >> > @@ -40,7 +40,7 @@ jobs: > >> >> >> > - env: DEF_LIB="static" > >> >> >> > arch: amd64 > >> >> >> > compiler: gcc > >> >> >> > - - env: DEF_LIB="shared" RUN_TESTS=1 > >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests > >> >> >> nohuge-tests" > >> >> >> > arch: amd64 > >> >> >> > compiler: gcc > >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -63,7 +63,7 @@ jobs: > >> >> >> > - env: DEF_LIB="static" > >> >> >> > arch: amd64 > >> >> >> > compiler: clang > >> >> >> > - - env: DEF_LIB="shared" RUN_TESTS=1 > >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests > >> >> >> nohuge-tests" > >> >> >> > arch: amd64 > >> >> >> > compiler: clang > >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -101,6 +101,9 @@ jobs: > >> >> >> > - env: DEF_LIB="static" > >> >> >> > arch: arm64 > >> >> >> > compiler: gcc > >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-tests" > >> >> >> > + arch: arm64 > >> >> >> > + compiler: gcc > >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 > >> >> >> > arch: arm64 > >> >> >> > compiler: gcc > >> >> >> > @@ -124,3 +127,6 @@ jobs: > >> >> >> > - env: DEF_LIB="shared" > >> >> >> > arch: arm64 > >> >> >> > compiler: clang > >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-tests" > >> >> >> > + arch: arm64 > >> >> >> > + compiler: clang > >> >> >> > diff --git a/app/test/meson.build b/app/test/meson.build index > >> >> >> > 0a2ce710f..162a1a76f 100644 > >> >> >> > --- a/app/test/meson.build > >> >> >> > +++ b/app/test/meson.build > >> >> >> > @@ -237,6 +237,60 @@ fast_test_names = [ > >> >> >> > 'thash_autotest', > >> >> >> > ] > >> >> >> > >> >> >> Shouldn't we also trim the list of fast-tests? Otherwise, these > >> >> >> tests will run twice. > >> >> >> > >> >> > I think you mean to have exclusive lists for fast-tests and nohuge-tests. > >> >> > >> >> That's what I was thinking. > >> >> > >> >> > Overlapped cases will run twice if both test suites are opted in. > >> >> > But the two runs are not the same, one runs with hugepage and the > >> >> > other runs in no-huge mode. > >> >> > >> >> Is it really so different between huge and no-huge? Most of the libraries > >> >> won't care - they call the rte_**alloc functions, and it returns blocks of > >> >> memory. Maybe I am simplifying it too much. > >> >> > >> >> > If fast-tests list is splited, we will need to always run multiple > >> >> > suites to cover all fast tests. > >> >> > We can keep x86 to run only fast-tests suite to avoid extra test runs > >> >> > if they are not necessary. Thoughts? > >> >> > >> >> I guess since most DPDK usage will be with hugepages, we should prefer to > >> >> test with it. I don't care too much about the color of this particular shed. If > >> >> you want to do it that way, it's okay by me - it gives us the coverage, and > >> >> doesn't duplicate tests between those environments. > >> >> > >> >> BUT it means when we add a new test to the suite, we need to remember to > >> >> add it in two places - fast_test and nohuge_test. That almost guarantees we > >> >> will miss tests (because we accidentally don't add it to nohuge). Maybe > >> >> there's another way, like keep a list of all the tests and some information on > >> >> whether the test needs hugepages to run. Then if there are no hugepages > >> >> available, we can write that we SKIP the tests that don't support huge pages. > >> >> In that way, we don't need two different lists - and if there are hugepages, > >> >> we will run all the test cases. > >> >> WDYT? > >> >> > >> > Yes. Agree with you that having duplicate tests in suites is error prone. > >> > >> Cool! > >> > >> > IIUC, cases in a suite is determined at build time, as well as command options to run cases. > >> > This implies hugepage availability needs to be detected at build time if we want to run only > >> > suitable cases in suite in an environment. It could be something we don't want. > >> > > >> > I'll trim fast-tests in next version to remove duplicated cases. > >> > >> I think it might be better to make the array something like (just a > >> psuedo-code example): > >> > >> # psuedo-code to check for hugepages > >> has_hugepages = check_for_hugepages() > >> > >> ... > >> > >> fast_test_names = [ > >> ['acl_autotest', true], > >> ['alarm_autotest', true], > >> ['atomic_autotest', true], > >> ... > >> > >> Then in the code: > >> > >> foreach arg : fast_test_names > >> .... > >> if not arg[1] > >> test(arg[0], ...) > >> if has_hugepages and arg[1] > >> test(arg[0], ) > >> > >> Does it make sense? Do you see a problem? > > > > I just hope meson won't create a dependency on hugepage availability... > > Thinking about stuff like: > > https://git.dpdk.org/dpdk/commit/?id=599d67b6a4bf96096352cc5fbc8fc28e54a1ca62 > > Yes, I agree. We probably need to do some kind of similar abstraction. > Hopefully not, since this is for run/test targets rather than for rebuilds. Try the simple approach first, and only add abstractions if it causes problems, I think.