* [dpdk-dev] [PATCH] test: rely on EAL detection for core list @ 2021-10-18 17:01 David Marchand 2021-10-18 17:53 ` Bruce Richardson 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand 0 siblings, 2 replies; 13+ messages in thread From: David Marchand @ 2021-10-18 17:01 UTC (permalink / raw) To: dev; +Cc: aconole, blo Cores count has a direct impact on the time needed to complete unit tests. Currently, the core list used for unit test is enforced to "all cores on the system" with no way for (CI) users to adapt it. On the other hand, EAL default behavior (when no -c/-l option gets passed) is to start threads on as many cores available in the process cpu affinity. Remove logic from meson: users can then select where to run the tests by either running meson with a custom cpu affinity (using taskset/cpuset depending on OS) or by passing a --test-args option to meson. Example: $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" Signed-off-by: David Marchand <david.marchand@redhat.com> --- I wanted to post this as a RFC, but now, I wonder if all CI test RFC patches, so sending as a normal patch. --- app/test/get-coremask.sh | 13 ------------- app/test/meson.build | 10 +--------- 2 files changed, 1 insertion(+), 22 deletions(-) delete mode 100755 app/test/get-coremask.sh diff --git a/app/test/get-coremask.sh b/app/test/get-coremask.sh deleted file mode 100755 index bb8cf404d2..0000000000 --- a/app/test/get-coremask.sh +++ /dev/null @@ -1,13 +0,0 @@ -#! /bin/sh -e -# SPDX-License-Identifier: BSD-3-Clause -# Copyright(c) 2019 Intel Corporation - -if [ "$(uname)" = "Linux" ] ; then - cat /sys/devices/system/cpu/present -elif [ "$(uname)" = "FreeBSD" ] ; then - ncpus=$(/sbin/sysctl -n hw.ncpu) - echo 0-$(expr $ncpus - 1) -else -# fallback - echo 0-3 -fi diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a1..c7b377d52d 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -463,13 +463,8 @@ message('hugepage availability: @0@'.format(has_hugepage)) timeout_seconds = 600 timeout_seconds_fast = 10 -get_coremask = find_program('get-coremask.sh') -num_cores_arg = '-l ' + run_command(get_coremask).stdout().strip() - -default_test_args = [num_cores_arg] - foreach arg : fast_tests - test_args = default_test_args + test_args = [] run_test = true if not has_hugepage if arg[1] @@ -502,7 +497,6 @@ endforeach foreach arg : perf_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'perf-tests') @@ -511,7 +505,6 @@ endforeach foreach arg : driver_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'driver-tests') @@ -520,7 +513,6 @@ endforeach foreach arg : dump_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'debug-tests') -- 2.23.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] test: rely on EAL detection for core list 2021-10-18 17:01 [dpdk-dev] [PATCH] test: rely on EAL detection for core list David Marchand @ 2021-10-18 17:53 ` Bruce Richardson 2021-10-19 9:52 ` David Marchand 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand 1 sibling, 1 reply; 13+ messages in thread From: Bruce Richardson @ 2021-10-18 17:53 UTC (permalink / raw) To: David Marchand; +Cc: dev, aconole, blo On Mon, Oct 18, 2021 at 07:01:36PM +0200, David Marchand wrote: > Cores count has a direct impact on the time needed to complete unit > tests. > > Currently, the core list used for unit test is enforced to "all cores on > the system" with no way for (CI) users to adapt it. > On the other hand, EAL default behavior (when no -c/-l option gets passed) > is to start threads on as many cores available in the process cpu > affinity. > > Remove logic from meson: users can then select where to run the tests by > either running meson with a custom cpu affinity (using taskset/cpuset > depending on OS) or by passing a --test-args option to meson. > > Example: > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > I wanted to post this as a RFC, but now, I wonder if all CI test RFC > patches, so sending as a normal patch. > > --- I really like this idea! Patch looks good other than it needs some doc changes. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] test: rely on EAL detection for core list 2021-10-18 17:53 ` Bruce Richardson @ 2021-10-19 9:52 ` David Marchand 2021-10-19 10:04 ` Bruce Richardson 0 siblings, 1 reply; 13+ messages in thread From: David Marchand @ 2021-10-19 9:52 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, Aaron Conole, Brandon Lo On Mon, Oct 18, 2021 at 7:53 PM Bruce Richardson <bruce.richardson@intel.com> wrote: > > On Mon, Oct 18, 2021 at 07:01:36PM +0200, David Marchand wrote: > > Cores count has a direct impact on the time needed to complete unit > > tests. > > > > Currently, the core list used for unit test is enforced to "all cores on > > the system" with no way for (CI) users to adapt it. > > On the other hand, EAL default behavior (when no -c/-l option gets passed) > > is to start threads on as many cores available in the process cpu > > affinity. > > > > Remove logic from meson: users can then select where to run the tests by > > either running meson with a custom cpu affinity (using taskset/cpuset > > depending on OS) or by passing a --test-args option to meson. > > > > Example: > > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > I wanted to post this as a RFC, but now, I wonder if all CI test RFC > > patches, so sending as a normal patch. > > > > --- > I really like this idea! Patch looks good other than it needs some doc > changes. Wdyt of: diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst index fff88655dd..d35e0577c8 100644 --- a/doc/guides/prog_guide/meson_ut.rst +++ b/doc/guides/prog_guide/meson_ut.rst @@ -47,9 +47,15 @@ Arguments of ``test()`` that can be provided in meson.build are as below: * ``is_parallel`` is used to run test case either in parallel or non-parallel mode. * ``timeout`` is used to specify the timeout of test case. -* ``args`` is used to specify test specific parameters. +* ``args`` is used to specify test specific parameters (see note below). * ``env`` is used to specify test specific environment parameters. +Note: the content of meson ``--test-args`` option and the content of ``args`` +are appended when invoking the DPDK test binary. +Because of this, it is recommended not to set any default coremask or memory +configuration in per test ``args`` and rather let users select what best fits +their environment. If a test can't run, then it should be skipped, as described +below. Dealing with skipped test cases ------------------------------- -- David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] test: rely on EAL detection for core list 2021-10-19 9:52 ` David Marchand @ 2021-10-19 10:04 ` Bruce Richardson 0 siblings, 0 replies; 13+ messages in thread From: Bruce Richardson @ 2021-10-19 10:04 UTC (permalink / raw) To: David Marchand; +Cc: dev, Aaron Conole, Brandon Lo On Tue, Oct 19, 2021 at 11:52:32AM +0200, David Marchand wrote: > On Mon, Oct 18, 2021 at 7:53 PM Bruce Richardson > <bruce.richardson@intel.com> wrote: > > > > On Mon, Oct 18, 2021 at 07:01:36PM +0200, David Marchand wrote: > > > Cores count has a direct impact on the time needed to complete unit > > > tests. > > > > > > Currently, the core list used for unit test is enforced to "all cores on > > > the system" with no way for (CI) users to adapt it. > > > On the other hand, EAL default behavior (when no -c/-l option gets passed) > > > is to start threads on as many cores available in the process cpu > > > affinity. > > > > > > Remove logic from meson: users can then select where to run the tests by > > > either running meson with a custom cpu affinity (using taskset/cpuset > > > depending on OS) or by passing a --test-args option to meson. > > > > > > Example: > > > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > > --- > > > I wanted to post this as a RFC, but now, I wonder if all CI test RFC > > > patches, so sending as a normal patch. > > > > > > --- > > I really like this idea! Patch looks good other than it needs some doc > > changes. > > Wdyt of: > > diff --git a/doc/guides/prog_guide/meson_ut.rst > b/doc/guides/prog_guide/meson_ut.rst > index fff88655dd..d35e0577c8 100644 > --- a/doc/guides/prog_guide/meson_ut.rst > +++ b/doc/guides/prog_guide/meson_ut.rst > @@ -47,9 +47,15 @@ Arguments of ``test()`` that can be provided in > meson.build are as below: > > * ``is_parallel`` is used to run test case either in parallel or > non-parallel mode. > * ``timeout`` is used to specify the timeout of test case. > -* ``args`` is used to specify test specific parameters. > +* ``args`` is used to specify test specific parameters (see note below). > * ``env`` is used to specify test specific environment parameters. > > +Note: the content of meson ``--test-args`` option and the content of ``args`` > +are appended when invoking the DPDK test binary. > +Because of this, it is recommended not to set any default coremask or memory > +configuration in per test ``args`` and rather let users select what best fits > +their environment. If a test can't run, then it should be skipped, as described > +below. > > Dealing with skipped test cases > ------------------------------- That text is good. However, I also think we need an example above of using test-args to limit core masks. How about also adding something like: diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst index fff88655dd..4924066556 100644 --- a/doc/guides/prog_guide/meson_ut.rst +++ b/doc/guides/prog_guide/meson_ut.rst @@ -37,6 +37,13 @@ command:: $ meson test --suite fast-tests +If desired, additional arguments can be passed to the test run via the meson ``--test-args`` option. +For example, tests will by default run on as many available cores as is needed for the test, +starting with the lowest number core - generally core 0. +To run the fast-tests suite using only cores 8 through 16, one can use:: + + $ meson test --suite fast-tests --test-args="-l 8-16" + The meson command to list all available tests:: $ meson test --list ^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-18 17:01 [dpdk-dev] [PATCH] test: rely on EAL detection for core list David Marchand 2021-10-18 17:53 ` Bruce Richardson @ 2021-10-19 11:26 ` David Marchand 2021-10-19 12:43 ` Bruce Richardson ` (2 more replies) 1 sibling, 3 replies; 13+ messages in thread From: David Marchand @ 2021-10-19 11:26 UTC (permalink / raw) To: dev; +Cc: aconole, blo, bruce.richardson Cores count has a direct impact on the time needed to complete unit tests. Currently, the core list used for unit test is enforced to "all cores on the system" with no way for (CI) users to adapt it. On the other hand, EAL default behavior (when no -c/-l option gets passed) is to start threads on as many cores available in the process cpu affinity. Remove logic from meson: users can then select where to run the tests by either running meson with a custom cpu affinity (using taskset/cpuset depending on OS) or by passing a --test-args option to meson. Example: $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" Signed-off-by: David Marchand <david.marchand@redhat.com> --- Changes since v1: - updated documentation, --- app/test/get-coremask.sh | 13 ------------- app/test/meson.build | 10 +--------- doc/guides/prog_guide/meson_ut.rst | 17 ++++++++++++++++- 3 files changed, 17 insertions(+), 23 deletions(-) delete mode 100755 app/test/get-coremask.sh diff --git a/app/test/get-coremask.sh b/app/test/get-coremask.sh deleted file mode 100755 index bb8cf404d2..0000000000 --- a/app/test/get-coremask.sh +++ /dev/null @@ -1,13 +0,0 @@ -#! /bin/sh -e -# SPDX-License-Identifier: BSD-3-Clause -# Copyright(c) 2019 Intel Corporation - -if [ "$(uname)" = "Linux" ] ; then - cat /sys/devices/system/cpu/present -elif [ "$(uname)" = "FreeBSD" ] ; then - ncpus=$(/sbin/sysctl -n hw.ncpu) - echo 0-$(expr $ncpus - 1) -else -# fallback - echo 0-3 -fi diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a1..c7b377d52d 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -463,13 +463,8 @@ message('hugepage availability: @0@'.format(has_hugepage)) timeout_seconds = 600 timeout_seconds_fast = 10 -get_coremask = find_program('get-coremask.sh') -num_cores_arg = '-l ' + run_command(get_coremask).stdout().strip() - -default_test_args = [num_cores_arg] - foreach arg : fast_tests - test_args = default_test_args + test_args = [] run_test = true if not has_hugepage if arg[1] @@ -502,7 +497,6 @@ endforeach foreach arg : perf_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'perf-tests') @@ -511,7 +505,6 @@ endforeach foreach arg : driver_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'driver-tests') @@ -520,7 +513,6 @@ endforeach foreach arg : dump_test_names test(arg, dpdk_test, env : ['DPDK_TEST=' + arg], - args : default_test_args, timeout : timeout_seconds, is_parallel : false, suite : 'debug-tests') diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst index fff88655dd..78cf3f845c 100644 --- a/doc/guides/prog_guide/meson_ut.rst +++ b/doc/guides/prog_guide/meson_ut.rst @@ -37,6 +37,14 @@ command:: $ meson test --suite fast-tests +If desired, additional arguments can be passed to the test run via the meson +``--test-args`` option. +For example, tests will by default run on as many available cores as is needed +for the test, starting with the lowest number core - generally core 0. +To run the fast-tests suite using only cores 8 through 16, one can use:: + + $ meson test --suite fast-tests --test-args="-l 8-16" + The meson command to list all available tests:: $ meson test --list @@ -47,9 +55,16 @@ Arguments of ``test()`` that can be provided in meson.build are as below: * ``is_parallel`` is used to run test case either in parallel or non-parallel mode. * ``timeout`` is used to specify the timeout of test case. -* ``args`` is used to specify test specific parameters. +* ``args`` is used to specify test specific parameters (see note below). * ``env`` is used to specify test specific environment parameters. +Note: the content of meson ``--test-args`` option and the content of ``args`` +are appended when invoking the DPDK test binary. +Because of this, it is recommended not to set any default coremask or memory +configuration in per test ``args`` and rather let users select what best fits +their environment. If a test can't run, then it should be skipped, as described +below. + Dealing with skipped test cases ------------------------------- -- 2.23.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand @ 2021-10-19 12:43 ` Bruce Richardson 2021-10-19 14:46 ` Honnappa Nagarahalli 2021-10-19 19:09 ` Aaron Conole 2 siblings, 0 replies; 13+ messages in thread From: Bruce Richardson @ 2021-10-19 12:43 UTC (permalink / raw) To: David Marchand; +Cc: dev, aconole, blo On Tue, Oct 19, 2021 at 01:26:02PM +0200, David Marchand wrote: > Cores count has a direct impact on the time needed to complete unit > tests. > > Currently, the core list used for unit test is enforced to "all cores on > the system" with no way for (CI) users to adapt it. > On the other hand, EAL default behavior (when no -c/-l option gets passed) > is to start threads on as many cores available in the process cpu > affinity. > > Remove logic from meson: users can then select where to run the tests by > either running meson with a custom cpu affinity (using taskset/cpuset > depending on OS) or by passing a --test-args option to meson. > > Example: > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- Tested using ring_perf_autotest. By default this ran perf tests on multiple threads, cores and numa nodes. Passing in `--test-args="-l 0-3"` the tests only ran on the multiple cores option as no threads on multiple numa nodes, or no threads sharing a core were present. Tested-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand 2021-10-19 12:43 ` Bruce Richardson @ 2021-10-19 14:46 ` Honnappa Nagarahalli 2021-10-19 15:04 ` David Marchand 2021-10-19 19:09 ` Aaron Conole 2 siblings, 1 reply; 13+ messages in thread From: Honnappa Nagarahalli @ 2021-10-19 14:46 UTC (permalink / raw) To: David Marchand, dev Cc: aconole, blo, bruce.richardson, nd, Honnappa Nagarahalli, nd <snip> > > Cores count has a direct impact on the time needed to complete unit tests. We also need to control the number of iterations with in the tests. We could add something like "-i low/medium/high". The test cases then can use this to decide on how many iterations to run. > > Currently, the core list used for unit test is enforced to "all cores on the > system" with no way for (CI) users to adapt it. > On the other hand, EAL default behavior (when no -c/-l option gets passed) is > to start threads on as many cores available in the process cpu affinity. > > Remove logic from meson: users can then select where to run the tests by > either running meson with a custom cpu affinity (using taskset/cpuset > depending on OS) or by passing a --test-args option to meson. > > Example: > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > Changes since v1: > - updated documentation, > > --- > app/test/get-coremask.sh | 13 ------------- > app/test/meson.build | 10 +--------- > doc/guides/prog_guide/meson_ut.rst | 17 ++++++++++++++++- > 3 files changed, 17 insertions(+), 23 deletions(-) delete mode 100755 > app/test/get-coremask.sh > > diff --git a/app/test/get-coremask.sh b/app/test/get-coremask.sh deleted file > mode 100755 index bb8cf404d2..0000000000 > --- a/app/test/get-coremask.sh > +++ /dev/null > @@ -1,13 +0,0 @@ > -#! /bin/sh -e > -# SPDX-License-Identifier: BSD-3-Clause -# Copyright(c) 2019 Intel > Corporation > - > -if [ "$(uname)" = "Linux" ] ; then > - cat /sys/devices/system/cpu/present > -elif [ "$(uname)" = "FreeBSD" ] ; then > - ncpus=$(/sbin/sysctl -n hw.ncpu) > - echo 0-$(expr $ncpus - 1) > -else > -# fallback > - echo 0-3 > -fi > diff --git a/app/test/meson.build b/app/test/meson.build index > a16374b7a1..c7b377d52d 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -463,13 +463,8 @@ message('hugepage availability: > @0@'.format(has_hugepage)) timeout_seconds = 600 timeout_seconds_fast > = 10 > > -get_coremask = find_program('get-coremask.sh') -num_cores_arg = '-l ' + > run_command(get_coremask).stdout().strip() > - > -default_test_args = [num_cores_arg] > - > foreach arg : fast_tests > - test_args = default_test_args > + test_args = [] > run_test = true > if not has_hugepage > if arg[1] > @@ -502,7 +497,6 @@ endforeach > foreach arg : perf_test_names > test(arg, dpdk_test, > env : ['DPDK_TEST=' + arg], > - args : default_test_args, > timeout : timeout_seconds, > is_parallel : false, > suite : 'perf-tests') > @@ -511,7 +505,6 @@ endforeach > foreach arg : driver_test_names > test(arg, dpdk_test, > env : ['DPDK_TEST=' + arg], > - args : default_test_args, > timeout : timeout_seconds, > is_parallel : false, > suite : 'driver-tests') > @@ -520,7 +513,6 @@ endforeach > foreach arg : dump_test_names > test(arg, dpdk_test, > env : ['DPDK_TEST=' + arg], > - args : default_test_args, > timeout : timeout_seconds, > is_parallel : false, > suite : 'debug-tests') > diff --git a/doc/guides/prog_guide/meson_ut.rst > b/doc/guides/prog_guide/meson_ut.rst > index fff88655dd..78cf3f845c 100644 > --- a/doc/guides/prog_guide/meson_ut.rst > +++ b/doc/guides/prog_guide/meson_ut.rst > @@ -37,6 +37,14 @@ command:: > > $ meson test --suite fast-tests > > +If desired, additional arguments can be passed to the test run via the > +meson ``--test-args`` option. > +For example, tests will by default run on as many available cores as is > +needed for the test, starting with the lowest number core - generally core 0. > +To run the fast-tests suite using only cores 8 through 16, one can use:: > + > + $ meson test --suite fast-tests --test-args="-l 8-16" > + > The meson command to list all available tests:: > > $ meson test --list > @@ -47,9 +55,16 @@ Arguments of ``test()`` that can be provided in > meson.build are as below: > > * ``is_parallel`` is used to run test case either in parallel or non-parallel mode. > * ``timeout`` is used to specify the timeout of test case. > -* ``args`` is used to specify test specific parameters. > +* ``args`` is used to specify test specific parameters (see note below). > * ``env`` is used to specify test specific environment parameters. > > +Note: the content of meson ``--test-args`` option and the content of > +``args`` are appended when invoking the DPDK test binary. > +Because of this, it is recommended not to set any default coremask or > +memory configuration in per test ``args`` and rather let users select > +what best fits their environment. If a test can't run, then it should > +be skipped, as described below. > + > > Dealing with skipped test cases > ------------------------------- > -- > 2.23.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 14:46 ` Honnappa Nagarahalli @ 2021-10-19 15:04 ` David Marchand 2021-10-19 18:04 ` Honnappa Nagarahalli 0 siblings, 1 reply; 13+ messages in thread From: David Marchand @ 2021-10-19 15:04 UTC (permalink / raw) To: Honnappa Nagarahalli; +Cc: dev, aconole, blo, bruce.richardson, nd On Tue, Oct 19, 2021 at 4:46 PM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote: > > <snip> > > > > > Cores count has a direct impact on the time needed to complete unit tests. > We also need to control the number of iterations with in the tests. > > We could add something like "-i low/medium/high". The test cases then can use this to decide on how many iterations to run. This patch hands control of parameters that affect all tests to the CI. But number of iterations is a per test thing. What would be the definition of an "iteration"? Something that must be done in a maximum of cycles.. ? This could be something to look at, but it goes beyond this patch. -- David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 15:04 ` David Marchand @ 2021-10-19 18:04 ` Honnappa Nagarahalli 0 siblings, 0 replies; 13+ messages in thread From: Honnappa Nagarahalli @ 2021-10-19 18:04 UTC (permalink / raw) To: David Marchand Cc: dev, aconole, blo, bruce.richardson, nd, Honnappa Nagarahalli, nd <snip> > > > > > > > > Cores count has a direct impact on the time needed to complete unit tests. > > We also need to control the number of iterations with in the tests. > > > > We could add something like "-i low/medium/high". The test cases then can > use this to decide on how many iterations to run. > > This patch hands control of parameters that affect all tests to the CI. > > But number of iterations is a per test thing. Agree, it is a per test thing. But, multiple test files run the test case in several iterations. So, multiple test cases can make use of the same input from the user. > What would be the definition of an "iteration"? > Something that must be done in a maximum of cycles.. ? You can look at test_atomic.c, we have the following #define #define N 1000000 It is used as follows (in the simplest case) to repeat a test: for (i = 0; i < N; i++) rte_atomic16_inc(&a16); The value 1000000 defines how long this test will take. In a CI environment, we want to reduce the time taken, whereas in a local lab machine, I might not care about the time it takes to run the test. This could be an input from the user which the test cases can make use of. For ex: for 'low', test case might set N = 10000 and so on. > > This could be something to look at, but it goes beyond this patch. Agreed, this can be a separate patch. > > > -- > David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand 2021-10-19 12:43 ` Bruce Richardson 2021-10-19 14:46 ` Honnappa Nagarahalli @ 2021-10-19 19:09 ` Aaron Conole 2021-10-19 19:28 ` David Marchand 2021-10-21 14:50 ` David Marchand 2 siblings, 2 replies; 13+ messages in thread From: Aaron Conole @ 2021-10-19 19:09 UTC (permalink / raw) To: David Marchand; +Cc: dev, blo, bruce.richardson, Honnappa Nagarahalli, nd David Marchand <david.marchand@redhat.com> writes: > Cores count has a direct impact on the time needed to complete unit > tests. > > Currently, the core list used for unit test is enforced to "all cores on > the system" with no way for (CI) users to adapt it. > On the other hand, EAL default behavior (when no -c/-l option gets passed) > is to start threads on as many cores available in the process cpu > affinity. > > Remove logic from meson: users can then select where to run the tests by > either running meson with a custom cpu affinity (using taskset/cpuset > depending on OS) or by passing a --test-args option to meson. > > Example: > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- LGTM - the spelling issue flagged seems to be a false positive. Acked-by: Aaron Conole <aconole@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 19:09 ` Aaron Conole @ 2021-10-19 19:28 ` David Marchand 2021-10-20 11:20 ` David Marchand 2021-10-21 14:50 ` David Marchand 1 sibling, 1 reply; 13+ messages in thread From: David Marchand @ 2021-10-19 19:28 UTC (permalink / raw) To: Aaron Conole, Brandon Lo, Lincoln Lavoie Cc: dev, Bruce Richardson, Honnappa Nagarahalli, nd, Thomas Monjalon On Tue, Oct 19, 2021 at 9:09 PM Aaron Conole <aconole@redhat.com> wrote: > > David Marchand <david.marchand@redhat.com> writes: > > > Cores count has a direct impact on the time needed to complete unit > > tests. > > > > Currently, the core list used for unit test is enforced to "all cores on > > the system" with no way for (CI) users to adapt it. > > On the other hand, EAL default behavior (when no -c/-l option gets passed) > > is to start threads on as many cores available in the process cpu > > affinity. > > > > Remove logic from meson: users can then select where to run the tests by > > either running meson with a custom cpu affinity (using taskset/cpuset > > depending on OS) or by passing a --test-args option to meson. > > > > Example: > > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > LGTM - the spelling issue flagged seems to be a false positive. The first spelling issue is fixed in main: https://git.dpdk.org/dpdk/commit/?id=d0db11a82609 The other three warnings (all of them about "test-args") are introduced by this patch, and this is a problem: once merged, I understand any following patch will get flagged as failing this check. There may be something better to do for the mid/long term, but the quicker is to add test-args to UNH dictionnary. Once done, I can merge this patch. Lincoln, Brandon? > > Acked-by: Aaron Conole <aconole@redhat.com> > Thanks for the review. -- David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 19:28 ` David Marchand @ 2021-10-20 11:20 ` David Marchand 0 siblings, 0 replies; 13+ messages in thread From: David Marchand @ 2021-10-20 11:20 UTC (permalink / raw) To: Aaron Conole, Brandon Lo, Lincoln Lavoie Cc: dev, Bruce Richardson, Honnappa Nagarahalli, nd, Thomas Monjalon On Tue, Oct 19, 2021 at 9:28 PM David Marchand <david.marchand@redhat.com> wrote: > The other three warnings (all of them about "test-args") are > introduced by this patch, and this is a problem: once merged, I > understand any following patch will get flagged as failing this check. > There may be something better to do for the mid/long term, but the > quicker is to add test-args to UNH dictionnary. > Once done, I can merge this patch. Nevermind, we have new warnings raised by this check. I must have overlooked some test report seeing how it comes from a patch I merged. > Lincoln, Brandon? This check seems picky, and rc1 is closer than ever. I'll ignore it for now. -- David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: rely on EAL detection for core list 2021-10-19 19:09 ` Aaron Conole 2021-10-19 19:28 ` David Marchand @ 2021-10-21 14:50 ` David Marchand 1 sibling, 0 replies; 13+ messages in thread From: David Marchand @ 2021-10-21 14:50 UTC (permalink / raw) To: David Marchand Cc: dev, Aaron Conole, Brandon Lo, Bruce Richardson, Honnappa Nagarahalli, nd On Tue, Oct 19, 2021 at 9:09 PM Aaron Conole <aconole@redhat.com> wrote: > > David Marchand <david.marchand@redhat.com> writes: > > > Cores count has a direct impact on the time needed to complete unit > > tests. > > > > Currently, the core list used for unit test is enforced to "all cores on > > the system" with no way for (CI) users to adapt it. > > On the other hand, EAL default behavior (when no -c/-l option gets passed) > > is to start threads on as many cores available in the process cpu > > affinity. > > > > Remove logic from meson: users can then select where to run the tests by > > either running meson with a custom cpu affinity (using taskset/cpuset > > depending on OS) or by passing a --test-args option to meson. > > > > Example: > > $ sudo meson test -C build --suite fast-tests -t 3 --test-args "-l 0-3" > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > Tested-by: Bruce Richardson <bruce.richardson@intel.com> > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > Acked-by: Aaron Conole <aconole@redhat.com> Applied, thanks. -- David Marchand ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-10-21 14:50 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-18 17:01 [dpdk-dev] [PATCH] test: rely on EAL detection for core list David Marchand 2021-10-18 17:53 ` Bruce Richardson 2021-10-19 9:52 ` David Marchand 2021-10-19 10:04 ` Bruce Richardson 2021-10-19 11:26 ` [dpdk-dev] [PATCH v2] " David Marchand 2021-10-19 12:43 ` Bruce Richardson 2021-10-19 14:46 ` Honnappa Nagarahalli 2021-10-19 15:04 ` David Marchand 2021-10-19 18:04 ` Honnappa Nagarahalli 2021-10-19 19:09 ` Aaron Conole 2021-10-19 19:28 ` David Marchand 2021-10-20 11:20 ` David Marchand 2021-10-21 14:50 ` David Marchand
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).