From: Amit Gupta <agupta3@marvell.com> V1 changes: * hash_readwrite and hash_readwrite lockfree meson test was taking longer time to complete. The test always get TIMEOUT. * hash readwrtie test is split into functional and perf tests and moved to dpdk fast and perf testsuite accordingly. * hash readwrite lockfree is moved to dpdk perf testsuite. Amit Gupta (2): test/meson: hash test split into shorter subtests test/meson: hash lf test moved to dpdk perf testsuite app/test/meson.build | 5 +- app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) -- 1.8.3.1
From: Amit Gupta <agupta3@marvell.com> hash_readwrite meson test was taking longer time to complete. The test always get TIMEOUT, hence test is split into functional and perf test. perf test is being moved under dpdk perf testsuites in meson build. Signed-off-by: Amit Gupta <agupta3@marvell.com> --- app/test/meson.build | 3 +- app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/app/test/meson.build b/app/test/meson.build index ec40943..94fd9f8 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -219,7 +219,7 @@ fast_test_names = [ 'distributor_autotest', 'eventdev_common_autotest', 'fbarray_autotest', - 'hash_readwrite_autotest', + 'hash_readwrite_func_autotest', 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', @@ -262,6 +262,7 @@ perf_test_names = [ 'stack_perf_autotest', 'stack_lf_perf_autotest', 'rand_perf_autotest', + 'hash_readwrite_perf_autotest', ] driver_test_names = [ diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c index 4376b09..c25e904 100644 --- a/app/test/test_hash_readwrite.c +++ b/app/test/test_hash_readwrite.c @@ -606,6 +606,150 @@ struct { } static int +test_hash_rw_perf_main(void) +{ + /* + * Variables used to choose different tests. + * use_htm indicates if hardware transactional memory should be used. + * reader_faster indicates if the reader threads should finish earlier + * than writer threads. This is to timing either reader threads or + * writer threads for performance numbers. + */ + int use_htm, reader_faster; + unsigned int i = 0, core_id = 0; + + if (rte_lcore_count() < 3) { + printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); + return TEST_SKIPPED; + } + + RTE_LCORE_FOREACH_SLAVE(core_id) { + slave_core_ids[i] = core_id; + i++; + } + + setlocale(LC_NUMERIC, ""); + + if (rte_tm_supported()) { + printf("Hardware transactional memory (lock elision) " + "is supported\n"); + + printf("Test read-write with Hardware transactional memory\n"); + + use_htm = 1; + + reader_faster = 1; + if (test_hash_readwrite_perf(&htm_results, use_htm, + reader_faster) < 0) + return -1; + + reader_faster = 0; + if (test_hash_readwrite_perf(&htm_results, use_htm, + reader_faster) < 0) + return -1; + } else { + printf("Hardware transactional memory (lock elision) " + "is NOT supported\n"); + } + + printf("Test read-write without Hardware transactional memory\n"); + use_htm = 0; + + reader_faster = 1; + if (test_hash_readwrite_perf(&non_htm_results, use_htm, + reader_faster) < 0) + return -1; + reader_faster = 0; + if (test_hash_readwrite_perf(&non_htm_results, use_htm, + reader_faster) < 0) + return -1; + + printf("================\n"); + printf("Results summary:\n"); + printf("================\n"); + + printf("single read: %u\n", htm_results.single_read); + printf("single write: %u\n", htm_results.single_write); + for (i = 0; i < NUM_TEST; i++) { + printf("+++ core_cnt: %u +++\n", core_cnt[i]); + printf("HTM:\n"); + printf(" read only: %u\n", htm_results.read_only[i]); + printf(" write only: %u\n", htm_results.write_only[i]); + printf(" read-write read: %u\n", htm_results.read_write_r[i]); + printf(" read-write write: %u\n", htm_results.read_write_w[i]); + + printf("non HTM:\n"); + printf(" read only: %u\n", non_htm_results.read_only[i]); + printf(" write only: %u\n", non_htm_results.write_only[i]); + printf(" read-write read: %u\n", + non_htm_results.read_write_r[i]); + printf(" read-write write: %u\n", + non_htm_results.read_write_w[i]); + } + + return 0; +} + +static int +test_hash_rw_func_main(void) +{ + /* + * Variables used to choose different tests. + * use_htm indicates if hardware transactional memory should be used. + * reader_faster indicates if the reader threads should finish earlier + * than writer threads. This is to timing either reader threads or + * writer threads for performance numbers. + */ + int use_htm, use_ext; + unsigned int i = 0, core_id = 0; + + if (rte_lcore_count() < 3) { + printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); + return TEST_SKIPPED; + } + + RTE_LCORE_FOREACH_SLAVE(core_id) { + slave_core_ids[i] = core_id; + i++; + } + + setlocale(LC_NUMERIC, ""); + + if (rte_tm_supported()) { + printf("Hardware transactional memory (lock elision) " + "is supported\n"); + + printf("Test read-write with Hardware transactional memory\n"); + + use_htm = 1; + use_ext = 0; + + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + use_ext = 1; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + } else { + printf("Hardware transactional memory (lock elision) " + "is NOT supported\n"); + } + + printf("Test read-write without Hardware transactional memory\n"); + use_htm = 0; + use_ext = 0; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + use_ext = 1; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + return 0; +} + +static int test_hash_readwrite_main(void) { /* @@ -706,3 +850,5 @@ struct { } REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main); +REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main); +REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main); -- 1.8.3.1
From: Amit Gupta <agupta3@marvell.com> hash_readwrite_lf test always getting TIMEOUT as required time to finish this test was much longer compare to time required for fast tests(10s). Hence, the test is being moved to perf test category for its execution to complete. Signed-off-by: Amit Gupta <agupta3@marvell.com> --- app/test/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/meson.build b/app/test/meson.build index 94fd9f8..34141c5 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -220,7 +220,6 @@ fast_test_names = [ 'eventdev_common_autotest', 'fbarray_autotest', 'hash_readwrite_func_autotest', - 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', 'kvargs_autotest', @@ -263,6 +262,7 @@ perf_test_names = [ 'stack_lf_perf_autotest', 'rand_perf_autotest', 'hash_readwrite_perf_autotest', + 'hash_readwrite_lf_autotest', ] driver_test_names = [ -- 1.8.3.1
Ping.. -----Original Message----- From: agupta3@marvell.com <agupta3@marvell.com> Sent: Friday, September 6, 2019 11:20 AM To: yipeng1.wang@intel.com; sameh.gobriel@intel.com; bruce.richardson@intel.com; pablo.de.lara.guarch@intel.com Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com> Subject: [PATCH 0/2] test/meson: fix hash readwrite timeout failure From: Amit Gupta <agupta3@marvell.com> V1 changes: * hash_readwrite and hash_readwrite lockfree meson test was taking longer time to complete. The test always get TIMEOUT. * hash readwrtie test is split into functional and perf tests and moved to dpdk fast and perf testsuite accordingly. * hash readwrite lockfree is moved to dpdk perf testsuite. Amit Gupta (2): test/meson: hash test split into shorter subtests test/meson: hash lf test moved to dpdk perf testsuite app/test/meson.build | 5 +- app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) -- 1.8.3.1
>-----Original Message-----
>From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>Sent: Thursday, September 5, 2019 10:50 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
>Subject: [PATCH 1/2] test/meson: hash test split into shorter subtests
>
>From: Amit Gupta <agupta3@marvell.com>
>
>hash_readwrite meson test was taking longer time to complete.
>The test always get TIMEOUT, hence test is split into
>functional and perf test. perf test is being moved under
>dpdk perf testsuites in meson build.
>
>Signed-off-by: Amit Gupta <agupta3@marvell.com>
[Wang, Yipeng]
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
Thanks for the patch!
>-----Original Message-----
>From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>Sent: Thursday, September 5, 2019 10:50 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
>Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
>
>From: Amit Gupta <agupta3@marvell.com>
>
>hash_readwrite_lf test always getting TIMEOUT as required
>time to finish this test was much longer compare to time
>required for fast tests(10s). Hence, the test is being
>moved to perf test category for its execution to complete.
>
>Signed-off-by: Amit Gupta <agupta3@marvell.com>
>---
> app/test/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/app/test/meson.build b/app/test/meson.build
>index 94fd9f8..34141c5 100644
>--- a/app/test/meson.build
>+++ b/app/test/meson.build
>@@ -220,7 +220,6 @@ fast_test_names = [
> 'eventdev_common_autotest',
> 'fbarray_autotest',
> 'hash_readwrite_func_autotest',
>- 'hash_readwrite_lf_autotest',
> 'ipsec_autotest',
> 'kni_autotest',
> 'kvargs_autotest',
>@@ -263,6 +262,7 @@ perf_test_names = [
> 'stack_lf_perf_autotest',
> 'rand_perf_autotest',
> 'hash_readwrite_perf_autotest',
>+ 'hash_readwrite_lf_autotest',
> ]
>
> driver_test_names = [
>--
>1.8.3.1
[Wang, Yipeng]
I believe the lf_autotest includes functional test as well which is critical for testing the lock free implementation on non-TSO machine.
Do you think it is possible to also separate this test?
I also include the ARM folks for opinions.
Thanks!
Yipeng
<snip> > >Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf > >testsuite > > > >From: Amit Gupta <agupta3@marvell.com> > > > >hash_readwrite_lf test always getting TIMEOUT as required time to > >finish this test was much longer compare to time required for fast > >tests(10s). Hence, the test is being moved to perf test category for > >its execution to complete. > > > >Signed-off-by: Amit Gupta <agupta3@marvell.com> > >--- > > app/test/meson.build | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/app/test/meson.build b/app/test/meson.build index > >94fd9f8..34141c5 100644 > >--- a/app/test/meson.build > >+++ b/app/test/meson.build > >@@ -220,7 +220,6 @@ fast_test_names = [ > > 'eventdev_common_autotest', > > 'fbarray_autotest', > > 'hash_readwrite_func_autotest', > >- 'hash_readwrite_lf_autotest', > > 'ipsec_autotest', > > 'kni_autotest', > > 'kvargs_autotest', > >@@ -263,6 +262,7 @@ perf_test_names = [ > > 'stack_lf_perf_autotest', > > 'rand_perf_autotest', > > 'hash_readwrite_perf_autotest', > >+ 'hash_readwrite_lf_autotest', > > ] > > > > driver_test_names = [ > >-- > >1.8.3.1 > [Wang, Yipeng] > I believe the lf_autotest includes functional test as well which is critical for > testing the lock free implementation on non-TSO machine. > Do you think it is possible to also separate this test? > I also include the ARM folks for opinions. Thanks Yipeng. We combined the functional testing with performance testing. So, it is all performance tests. From that perspective, I suggest changing 'hash_readwrite_lf_autotest' to 'hash_readwrite_lf_perf_autotest'. > > Thanks! > Yipeng
From: Amit Gupta <agupta3@marvell.com> hash_readwrite_lf test always getting TIMEOUT as required time to finish this test was much longer compare to time required for fast tests(10s). Hence, the test is being renamed moved to perf test category for its execution to complete. Signed-off-by: Amit Gupta <agupta3@marvell.com> --- app/test/meson.build | 2 +- app/test/test_hash_readwrite_lf.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/test/meson.build b/app/test/meson.build index 94fd9f8..57d5316 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -220,7 +220,6 @@ fast_test_names = [ 'eventdev_common_autotest', 'fbarray_autotest', 'hash_readwrite_func_autotest', - 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', 'kvargs_autotest', @@ -263,6 +262,7 @@ perf_test_names = [ 'stack_lf_perf_autotest', 'rand_perf_autotest', 'hash_readwrite_perf_autotest', + 'hash_readwrite_lf_perf_autotest', ] driver_test_names = [ diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c index 1f2fba4..33d63fa 100644 --- a/app/test/test_hash_readwrite_lf.c +++ b/app/test/test_hash_readwrite_lf.c @@ -1431,4 +1431,5 @@ struct { return 0; } -REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main); +REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest, + test_hash_readwrite_lf_main); -- 1.8.3.1
From: Amit Gupta <agupta3@marvell.com> hash_readwrite_lf test always getting TIMEOUT as required time to finish this test was much longer compare to time required for fast tests(10s). Hence, the test is being renamed moved to perf test category for its execution to complete. Signed-off-by: Amit Gupta <agupta3@marvell.com> --- v2 Changes: *hash_readwrite_lf_autotest renamed to hash_readwrite_lf_pref_autotest app/test/meson.build | 2 +- app/test/test_hash_readwrite_lf.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/test/meson.build b/app/test/meson.build index 94fd9f8..57d5316 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -220,7 +220,6 @@ fast_test_names = [ 'eventdev_common_autotest', 'fbarray_autotest', 'hash_readwrite_func_autotest', - 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', 'kvargs_autotest', @@ -263,6 +262,7 @@ perf_test_names = [ 'stack_lf_perf_autotest', 'rand_perf_autotest', 'hash_readwrite_perf_autotest', + 'hash_readwrite_lf_perf_autotest', ] driver_test_names = [ diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c index 1f2fba4..33d63fa 100644 --- a/app/test/test_hash_readwrite_lf.c +++ b/app/test/test_hash_readwrite_lf.c @@ -1431,4 +1431,5 @@ struct { return 0; } -REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main); +REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest, + test_hash_readwrite_lf_main); -- 1.8.3.1
> -----Original Message----- > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> > Sent: Thursday, September 12, 2019 8:31 PM > To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta > <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; > Dharmik Thakkar <Dharmik.Thakkar@arm.com>; Ruifeng Wang (Arm > Technology China) <Ruifeng.Wang@arm.com> > Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli > <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com> > Subject: [EXT] RE: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf > testsuite > > External Email > > ---------------------------------------------------------------------- > <snip> > > > >Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf > > >testsuite > > > > > >From: Amit Gupta <agupta3@marvell.com> > > > > > >hash_readwrite_lf test always getting TIMEOUT as required time to > > >finish this test was much longer compare to time required for fast > > >tests(10s). Hence, the test is being moved to perf test category for > > >its execution to complete. > > > > > >Signed-off-by: Amit Gupta <agupta3@marvell.com> > > >--- > > > app/test/meson.build | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > >diff --git a/app/test/meson.build b/app/test/meson.build index > > >94fd9f8..34141c5 100644 > > >--- a/app/test/meson.build > > >+++ b/app/test/meson.build > > >@@ -220,7 +220,6 @@ fast_test_names = [ > > > 'eventdev_common_autotest', > > > 'fbarray_autotest', > > > 'hash_readwrite_func_autotest', > > >- 'hash_readwrite_lf_autotest', > > > 'ipsec_autotest', > > > 'kni_autotest', > > > 'kvargs_autotest', > > >@@ -263,6 +262,7 @@ perf_test_names = [ > > > 'stack_lf_perf_autotest', > > > 'rand_perf_autotest', > > > 'hash_readwrite_perf_autotest', > > >+ 'hash_readwrite_lf_autotest', > > > ] > > > > > > driver_test_names = [ > > >-- > > >1.8.3.1 > > [Wang, Yipeng] > > I believe the lf_autotest includes functional test as well which is > > critical for testing the lock free implementation on non-TSO machine. > > Do you think it is possible to also separate this test? > > I also include the ARM folks for opinions. > Thanks Yipeng. We combined the functional testing with performance > testing. So, it is all performance tests. From that perspective, I suggest > changing 'hash_readwrite_lf_autotest' to > 'hash_readwrite_lf_perf_autotest'. Agreed, will update in v2! > > > > > Thanks! > > Yipeng
<agupta3@marvell.com> writes: > From: Amit Gupta <agupta3@marvell.com> > > hash_readwrite_lf test always getting TIMEOUT as required > time to finish this test was much longer compare to time > required for fast tests(10s). Hence, the test is being renamed > moved to perf test category for its execution to complete. > > Signed-off-by: Amit Gupta <agupta3@marvell.com> > --- Okay. I'll note that we pass the '-t 3' flag, so it is actually timing out with 30s instead of the default 10. We do this because occasionally the lpm6 and table tests would also exceed the 10s timeout in the travis environment. I agree, it's better to pull the perf part of tests out. I think there isn't any additional functional test in this readwrite - is that so? If it is, then we need to also prioritize adding back in some of the functional testing. Maybe I misread the lf_autotest, though. > app/test/meson.build | 2 +- > app/test/test_hash_readwrite_lf.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/app/test/meson.build b/app/test/meson.build > index 94fd9f8..57d5316 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -220,7 +220,6 @@ fast_test_names = [ > 'eventdev_common_autotest', > 'fbarray_autotest', > 'hash_readwrite_func_autotest', > - 'hash_readwrite_lf_autotest', > 'ipsec_autotest', > 'kni_autotest', > 'kvargs_autotest', > @@ -263,6 +262,7 @@ perf_test_names = [ > 'stack_lf_perf_autotest', > 'rand_perf_autotest', > 'hash_readwrite_perf_autotest', > + 'hash_readwrite_lf_perf_autotest', > ] > > driver_test_names = [ > diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c > index 1f2fba4..33d63fa 100644 > --- a/app/test/test_hash_readwrite_lf.c > +++ b/app/test/test_hash_readwrite_lf.c > @@ -1431,4 +1431,5 @@ struct { > return 0; > } > > -REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main); > +REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest, > + test_hash_readwrite_lf_main);
> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Friday, September 13, 2019 7:41 AM
> To: agupta3@marvell.com
> Cc: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to
> dpdk perf testsuite
>
> <agupta3@marvell.com> writes:
>
> > From: Amit Gupta <agupta3@marvell.com>
> >
> > hash_readwrite_lf test always getting TIMEOUT as required time to
> > finish this test was much longer compare to time required for fast
> > tests(10s). Hence, the test is being renamed moved to perf test
> > category for its execution to complete.
> >
> > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > ---
>
> Okay. I'll note that we pass the '-t 3' flag, so it is actually timing out with 30s
> instead of the default 10. We do this because occasionally the lpm6 and table
> tests would also exceed the 10s timeout in the travis environment. I agree,
> it's better to pull the perf part of tests out.
>
> I think there isn't any additional functional test in this readwrite - is that so?
> If it is, then we need to also prioritize adding back in some of the functional
> testing. Maybe I misread the lf_autotest, though.
>
[Wang, Yipeng]
Yes that is my concern too, if we just move all the lock-free test into perf test then we miss
the functional test.
Would any of you like to consider adding a small functional test into the readwrite or readwrite_lf_functional?
That would be great :)
<snip> > > > > <agupta3@marvell.com> writes: > > > > > From: Amit Gupta <agupta3@marvell.com> > > > > > > hash_readwrite_lf test always getting TIMEOUT as required time to > > > finish this test was much longer compare to time required for fast > > > tests(10s). Hence, the test is being renamed moved to perf test > > > category for its execution to complete. > > > > > > Signed-off-by: Amit Gupta <agupta3@marvell.com> > > > --- > > > > Okay. I'll note that we pass the '-t 3' flag, so it is actually > > timing out with 30s instead of the default 10. We do this because > > occasionally the lpm6 and table tests would also exceed the 10s > > timeout in the travis environment. I agree, it's better to pull the perf part > of tests out. > > > > I think there isn't any additional functional test in this readwrite - is that so? > > If it is, then we need to also prioritize adding back in some of the > > functional testing. Maybe I misread the lf_autotest, though. > > > [Wang, Yipeng] > Yes that is my concern too, if we just move all the lock-free test into perf test > then we miss the functional test. > Would any of you like to consider adding a small functional test into the > readwrite or readwrite_lf_functional? > That would be great :) Yes, I will take up for readwrite_lf_functional. But, I do not have much bandwidth for 19.11. I suggest we move only part of the tests to perf tests instead for 19.11, this would serve both the purposes. Amit, would it be possible to check what tests will run within the timeout period? >
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Friday, September 13, 2019 9:17 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> to dpdk perf testsuite
>
> External Email
>
> ----------------------------------------------------------------------
> <snip>
>
> > >
> > > <agupta3@marvell.com> writes:
> > >
> > > > From: Amit Gupta <agupta3@marvell.com>
> > > >
> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> > > > finish this test was much longer compare to time required for fast
> > > > tests(10s). Hence, the test is being renamed moved to perf test
> > > > category for its execution to complete.
> > > >
> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > > > ---
> > >
> > > Okay. I'll note that we pass the '-t 3' flag, so it is actually
> > > timing out with 30s instead of the default 10. We do this because
> > > occasionally the lpm6 and table tests would also exceed the 10s
> > > timeout in the travis environment. I agree, it's better to pull the
> > > perf part
> > of tests out.
> > >
> > > I think there isn't any additional functional test in this readwrite - is that
> so?
> > > If it is, then we need to also prioritize adding back in some of the
> > > functional testing. Maybe I misread the lf_autotest, though.
> > >
> > [Wang, Yipeng]
> > Yes that is my concern too, if we just move all the lock-free test
> > into perf test then we miss the functional test.
> > Would any of you like to consider adding a small functional test into
> > the readwrite or readwrite_lf_functional?
> > That would be great :)
> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> instead for 19.11, this would serve both the purposes.
>
> Amit, would it be possible to check what tests will run within the timeout
> period?
> >
Not even a single subtest of hash_readwrite_lf runs within the timeout period of 10s.
generate_key logic itself is taking ~8s to generate all required keys.
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Friday, September 13, 2019 9:17 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> to dpdk perf testsuite
>
> External Email
>
> ----------------------------------------------------------------------
> <snip>
>
> > >
> > > <agupta3@marvell.com> writes:
> > >
> > > > From: Amit Gupta <agupta3@marvell.com>
> > > >
> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> > > > finish this test was much longer compare to time required for fast
> > > > tests(10s). Hence, the test is being renamed moved to perf test
> > > > category for its execution to complete.
> > > >
> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > > > ---
> > >
> > > Okay. I'll note that we pass the '-t 3' flag, so it is actually
> > > timing out with 30s instead of the default 10. We do this because
> > > occasionally the lpm6 and table tests would also exceed the 10s
> > > timeout in the travis environment. I agree, it's better to pull the
> > > perf part
> > of tests out.
> > >
> > > I think there isn't any additional functional test in this readwrite - is that
> so?
> > > If it is, then we need to also prioritize adding back in some of the
> > > functional testing. Maybe I misread the lf_autotest, though.
> > >
> > [Wang, Yipeng]
> > Yes that is my concern too, if we just move all the lock-free test
> > into perf test then we miss the functional test.
> > Would any of you like to consider adding a small functional test into
> > the readwrite or readwrite_lf_functional?
> > That would be great :)
> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> instead for 19.11, this would serve both the purposes.
>
> Amit, would it be possible to check what tests will run within the timeout
> period?
> >
@Wang, Yipeng1, is it good if we do the change as @Honnappa Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to 'hash_readwrite_lf_perf_autotest' for the time being and later once have sufficient bandwidth we can move only perf part of the test to perf tests.
> -----Original Message-----
> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> Sent: Wednesday, September 11, 2019 10:35 PM
> To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh
> <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into shorter
> subtests
>
> External Email
>
> ----------------------------------------------------------------------
> >-----Original Message-----
> >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> >Sent: Thursday, September 5, 2019 10:50 PM
> >To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> ><sameh.gobriel@intel.com>; Richardson, Bruce
> ><bruce.richardson@intel.com>; De Lara Guarch, Pablo
> ><pablo.de.lara.guarch@intel.com>
> >Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
> >Subject: [PATCH 1/2] test/meson: hash test split into shorter subtests
> >
> >From: Amit Gupta <agupta3@marvell.com>
> >
> >hash_readwrite meson test was taking longer time to complete.
> >The test always get TIMEOUT, hence test is split into functional and
> >perf test. perf test is being moved under dpdk perf testsuites in
> >meson build.
> >
> >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> [Wang, Yipeng]
> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
>
>
> Thanks for the patch!
@Wang, Yipeng1, any plan on taking this patch ?
Regards,
Amit
Amit Gupta <agupta3@marvell.com> writes:
>> -----Original Message-----
>> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>> Sent: Friday, September 13, 2019 9:17 PM
>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
>> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
>> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
>> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
>> to dpdk perf testsuite
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> <snip>
>>
>> > >
>> > > <agupta3@marvell.com> writes:
>> > >
>> > > > From: Amit Gupta <agupta3@marvell.com>
>> > > >
>> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
>> > > > finish this test was much longer compare to time required for fast
>> > > > tests(10s). Hence, the test is being renamed moved to perf test
>> > > > category for its execution to complete.
>> > > >
>> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
>> > > > ---
>> > >
>> > > Okay. I'll note that we pass the '-t 3' flag, so it is actually
>> > > timing out with 30s instead of the default 10. We do this because
>> > > occasionally the lpm6 and table tests would also exceed the 10s
>> > > timeout in the travis environment. I agree, it's better to pull the
>> > > perf part
>> > of tests out.
>> > >
>> > > I think there isn't any additional functional test in this readwrite - is that
>> so?
>> > > If it is, then we need to also prioritize adding back in some of the
>> > > functional testing. Maybe I misread the lf_autotest, though.
>> > >
>> > [Wang, Yipeng]
>> > Yes that is my concern too, if we just move all the lock-free test
>> > into perf test then we miss the functional test.
>> > Would any of you like to consider adding a small functional test into
>> > the readwrite or readwrite_lf_functional?
>> > That would be great :)
>> Yes, I will take up for readwrite_lf_functional. But, I do not have much
>> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
>> instead for 19.11, this would serve both the purposes.
>>
>> Amit, would it be possible to check what tests will run within the timeout
>> period?
>> >
> @Wang, Yipeng1, is it good if we do the change as @Honnappa
> Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to
> 'hash_readwrite_lf_perf_autotest' for the time being and later once
> have sufficient bandwidth we can move only perf part of the test to
> perf tests.
NAK.
I don't like that proposal. While it's true that there are occasional
TIMEOUT failures with the current setup, I'd much prefer these timeouts
(which we can easily distinguish) to removing the test from the travis
chain. My understanding is that there *are* some functionality being
exercised by this test that isn't exercised elsewhere. I'd prefer we
don't sacrifice the coverage.
On Thu, Oct 17, 2019 at 3:16 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Amit Gupta <agupta3@marvell.com> writes:
>
> >> -----Original Message-----
> >> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> >> Sent: Friday, September 13, 2019 9:17 PM
> >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> >> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> >> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> >> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> >> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> >> to dpdk perf testsuite
> >>
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> <snip>
> >>
> >> > >
> >> > > <agupta3@marvell.com> writes:
> >> > >
> >> > > > From: Amit Gupta <agupta3@marvell.com>
> >> > > >
> >> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> >> > > > finish this test was much longer compare to time required for fast
> >> > > > tests(10s). Hence, the test is being renamed moved to perf test
> >> > > > category for its execution to complete.
> >> > > >
> >> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >> > > > ---
> >> > >
> >> > > Okay. I'll note that we pass the '-t 3' flag, so it is actually
> >> > > timing out with 30s instead of the default 10. We do this because
> >> > > occasionally the lpm6 and table tests would also exceed the 10s
> >> > > timeout in the travis environment. I agree, it's better to pull the
> >> > > perf part
> >> > of tests out.
> >> > >
> >> > > I think there isn't any additional functional test in this readwrite - is that
> >> so?
> >> > > If it is, then we need to also prioritize adding back in some of the
> >> > > functional testing. Maybe I misread the lf_autotest, though.
> >> > >
> >> > [Wang, Yipeng]
> >> > Yes that is my concern too, if we just move all the lock-free test
> >> > into perf test then we miss the functional test.
> >> > Would any of you like to consider adding a small functional test into
> >> > the readwrite or readwrite_lf_functional?
> >> > That would be great :)
> >> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> >> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> >> instead for 19.11, this would serve both the purposes.
> >>
> >> Amit, would it be possible to check what tests will run within the timeout
> >> period?
> >> >
> > @Wang, Yipeng1, is it good if we do the change as @Honnappa
> > Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to
> > 'hash_readwrite_lf_perf_autotest' for the time being and later once
> > have sufficient bandwidth we can move only perf part of the test to
> > perf tests.
>
> NAK.
>
> I don't like that proposal. While it's true that there are occasional
> TIMEOUT failures with the current setup, I'd much prefer these timeouts
> (which we can easily distinguish) to removing the test from the travis
> chain. My understanding is that there *are* some functionality being
> exercised by this test that isn't exercised elsewhere. I'd prefer we
> don't sacrifice the coverage.
+1 and marking this patch as rejected.
On a sidenote, Amit, please be careful about the versioning of your
patches and update their status in patchwork.
I had two patches named the same with one marked as NEW (but no
comment on it) and this current thread patch marked as SUPERSEDED.
Thanks.
--
David Marchand
Ping!
> -----Original Message-----
> From: Amit Gupta
> Sent: Thursday, October 17, 2019 10:33 AM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>
>
>
> > -----Original Message-----
> > From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> > Sent: Wednesday, September 11, 2019 10:35 PM
> > To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh
> > <sameh.gobriel@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> > shorter subtests
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > >-----Original Message-----
> > >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> > >Sent: Thursday, September 5, 2019 10:50 PM
> > >To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> > ><sameh.gobriel@intel.com>; Richardson, Bruce
> > ><bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > ><pablo.de.lara.guarch@intel.com>
> > >Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
> > >Subject: [PATCH 1/2] test/meson: hash test split into shorter
> > >subtests
> > >
> > >From: Amit Gupta <agupta3@marvell.com>
> > >
> > >hash_readwrite meson test was taking longer time to complete.
> > >The test always get TIMEOUT, hence test is split into functional and
> > >perf test. perf test is being moved under dpdk perf testsuites in
> > >meson build.
> > >
> > >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > [Wang, Yipeng]
> > Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> >
> >
> > Thanks for the patch!
>
> @Wang, Yipeng1, any plan on taking this patch ?
>
>
> Regards,
> Amit
Hi, Amit,
I think I acked this patch. But from patchwork seems you superseded this patch set accidentally. So Thomas might have missed it.
To make his life easier, you may submit a newer version with my acked. I believe Thomas will see it.
Thanks
Yipeng
>-----Original Message-----
>From: Amit Gupta [mailto:agupta3@marvell.com]
>Sent: Thursday, October 31, 2019 9:54 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Cc: dev@dpdk.org
>Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>
>Ping!
>
>> -----Original Message-----
>> From: Amit Gupta
>> Sent: Thursday, October 17, 2019 10:33 AM
>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
>> <sameh.gobriel@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org
>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>>
>>
>>
>> > -----Original Message-----
>> > From: Wang, Yipeng1 <yipeng1.wang@intel.com>
>> > Sent: Wednesday, September 11, 2019 10:35 PM
>> > To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh
>> > <sameh.gobriel@intel.com>; Richardson, Bruce
>> > <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> > <pablo.de.lara.guarch@intel.com>
>> > Cc: dev@dpdk.org
>> > Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
>> > shorter subtests
>> >
>> > External Email
>> >
>> > ----------------------------------------------------------------------
>> > >-----Original Message-----
>> > >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>> > >Sent: Thursday, September 5, 2019 10:50 PM
>> > >To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
>> > ><sameh.gobriel@intel.com>; Richardson, Bruce
>> > ><bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> > ><pablo.de.lara.guarch@intel.com>
>> > >Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
>> > >Subject: [PATCH 1/2] test/meson: hash test split into shorter
>> > >subtests
>> > >
>> > >From: Amit Gupta <agupta3@marvell.com>
>> > >
>> > >hash_readwrite meson test was taking longer time to complete.
>> > >The test always get TIMEOUT, hence test is split into functional and
>> > >perf test. perf test is being moved under dpdk perf testsuites in
>> > >meson build.
>> > >
>> > >Signed-off-by: Amit Gupta <agupta3@marvell.com>
>> > [Wang, Yipeng]
>> > Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
>> >
>> >
>> > Thanks for the patch!
>>
>> @Wang, Yipeng1, any plan on taking this patch ?
>>
>>
>> Regards,
>> Amit
On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote: > Hi, Amit, > > I think I acked this patch. But from patchwork seems you superseded this patch set accidentally. So Thomas might have missed it. > To make his life easier, you may submit a newer version with my acked. I believe Thomas will see it. cc'ed David. Hi Amit, The patch is in "Superseded" state as Yipeng said, as far as I understand that is not the case, I am updating their status as "New". Can you please confirm this is the correct state? > > Thanks > Yipeng > >> -----Original Message----- >> From: Amit Gupta [mailto:agupta3@marvell.com] >> Sent: Thursday, October 31, 2019 9:54 PM >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Jerin Jacob Kollanukkaran >> <jerinj@marvell.com> >> Cc: dev@dpdk.org >> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests >> >> Ping! >> >>> -----Original Message----- >>> From: Amit Gupta >>> Sent: Thursday, October 17, 2019 10:33 AM >>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh >>> <sameh.gobriel@intel.com>; Richardson, Bruce >>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo >>> <pablo.de.lara.guarch@intel.com> >>> Cc: dev@dpdk.org >>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests >>> >>> >>> >>>> -----Original Message----- >>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com> >>>> Sent: Wednesday, September 11, 2019 10:35 PM >>>> To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh >>>> <sameh.gobriel@intel.com>; Richardson, Bruce >>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo >>>> <pablo.de.lara.guarch@intel.com> >>>> Cc: dev@dpdk.org >>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into >>>> shorter subtests >>>> >>>> External Email >>>> >>>> ---------------------------------------------------------------------- >>>>> -----Original Message----- >>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com] >>>>> Sent: Thursday, September 5, 2019 10:50 PM >>>>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh >>>>> <sameh.gobriel@intel.com>; Richardson, Bruce >>>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo >>>>> <pablo.de.lara.guarch@intel.com> >>>>> Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com> >>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter >>>>> subtests >>>>> >>>>> From: Amit Gupta <agupta3@marvell.com> >>>>> >>>>> hash_readwrite meson test was taking longer time to complete. >>>>> The test always get TIMEOUT, hence test is split into functional and >>>>> perf test. perf test is being moved under dpdk perf testsuites in >>>>> meson build. >>>>> >>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com> >>>> [Wang, Yipeng] >>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com> >>>> >>>> >>>> Thanks for the patch! >>> >>> @Wang, Yipeng1, any plan on taking this patch ? >>> >>> >>> Regards, >>> Amit
Hi Yipeng,
Thanks for changing the state to 'New'. Yes this patch was wrongly Superseded.
Regards,
Amit
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, November 5, 2019 10:07 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta
> <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into
> shorter subtests
>
> External Email
>
> ----------------------------------------------------------------------
> On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote:
> > Hi, Amit,
> >
> > I think I acked this patch. But from patchwork seems you superseded this
> patch set accidentally. So Thomas might have missed it.
> > To make his life easier, you may submit a newer version with my acked. I
> believe Thomas will see it.
>
> cc'ed David.
>
> Hi Amit,
>
> The patch is in "Superseded" state as Yipeng said, as far as I understand that
> is not the case, I am updating their status as "New". Can you please confirm
> this is the correct state?
>
> >
> > Thanks
> > Yipeng
> >
> >> -----Original Message-----
> >> From: Amit Gupta [mailto:agupta3@marvell.com]
> >> Sent: Thursday, October 31, 2019 9:54 PM
> >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> >> <sameh.gobriel@intel.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>
> >> Cc: dev@dpdk.org
> >> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> >> subtests
> >>
> >> Ping!
> >>
> >>> -----Original Message-----
> >>> From: Amit Gupta
> >>> Sent: Thursday, October 17, 2019 10:33 AM
> >>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> >>> <sameh.gobriel@intel.com>; Richardson, Bruce
> >>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >>> <pablo.de.lara.guarch@intel.com>
> >>> Cc: dev@dpdk.org
> >>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> >>> subtests
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> >>>> Sent: Wednesday, September 11, 2019 10:35 PM
> >>>> To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh
> >>>> <sameh.gobriel@intel.com>; Richardson, Bruce
> >>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >>>> <pablo.de.lara.guarch@intel.com>
> >>>> Cc: dev@dpdk.org
> >>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> >>>> shorter subtests
> >>>>
> >>>> External Email
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> ---
> >>>>> -----Original Message-----
> >>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> >>>>> Sent: Thursday, September 5, 2019 10:50 PM
> >>>>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> >>>>> <sameh.gobriel@intel.com>; Richardson, Bruce
> >>>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >>>>> <pablo.de.lara.guarch@intel.com>
> >>>>> Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
> >>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter
> >>>>> subtests
> >>>>>
> >>>>> From: Amit Gupta <agupta3@marvell.com>
> >>>>>
> >>>>> hash_readwrite meson test was taking longer time to complete.
> >>>>> The test always get TIMEOUT, hence test is split into functional
> >>>>> and perf test. perf test is being moved under dpdk perf testsuites
> >>>>> in meson build.
> >>>>>
> >>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >>>> [Wang, Yipeng]
> >>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> >>>>
> >>>>
> >>>> Thanks for the patch!
> >>>
> >>> @Wang, Yipeng1, any plan on taking this patch ?
> >>>
> >>>
> >>> Regards,
> >>> Amit
Ping!
Any plan to merge this change ?
Regards,
Amit
> -----Original Message-----
> From: Amit Gupta
> Sent: Thursday, November 7, 2019 9:03 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> Subject: RE: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split
> into shorter subtests
>
> Hi Yipeng,
>
> Thanks for changing the state to 'New'. Yes this patch was wrongly
> Superseded.
>
>
> Regards,
> Amit
>
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Tuesday, November 5, 2019 10:07 PM
> > To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta
> > <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> > Thomas Monjalon <thomas@monjalon.net>
> > Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> > Subject: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split
> > into shorter subtests
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote:
> > > Hi, Amit,
> > >
> > > I think I acked this patch. But from patchwork seems you superseded
> > > this
> > patch set accidentally. So Thomas might have missed it.
> > > To make his life easier, you may submit a newer version with my
> > > acked. I
> > believe Thomas will see it.
> >
> > cc'ed David.
> >
> > Hi Amit,
> >
> > The patch is in "Superseded" state as Yipeng said, as far as I
> > understand that is not the case, I am updating their status as "New".
> > Can you please confirm this is the correct state?
> >
> > >
> > > Thanks
> > > Yipeng
> > >
> > >> -----Original Message-----
> > >> From: Amit Gupta [mailto:agupta3@marvell.com]
> > >> Sent: Thursday, October 31, 2019 9:54 PM
> > >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> > >> <sameh.gobriel@intel.com>; Richardson, Bruce
> > >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > >> <pablo.de.lara.guarch@intel.com>; Jerin Jacob Kollanukkaran
> > >> <jerinj@marvell.com>
> > >> Cc: dev@dpdk.org
> > >> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> > >> subtests
> > >>
> > >> Ping!
> > >>
> > >>> -----Original Message-----
> > >>> From: Amit Gupta
> > >>> Sent: Thursday, October 17, 2019 10:33 AM
> > >>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> > >>> <sameh.gobriel@intel.com>; Richardson, Bruce
> > >>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > >>> <pablo.de.lara.guarch@intel.com>
> > >>> Cc: dev@dpdk.org
> > >>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> > >>> subtests
> > >>>
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> > >>>> Sent: Wednesday, September 11, 2019 10:35 PM
> > >>>> To: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh
> > >>>> <sameh.gobriel@intel.com>; Richardson, Bruce
> > >>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > >>>> <pablo.de.lara.guarch@intel.com>
> > >>>> Cc: dev@dpdk.org
> > >>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> > >>>> shorter subtests
> > >>>>
> > >>>> External Email
> > >>>>
> > >>>> -----------------------------------------------------------------
> > >>>> --
> > >>>> ---
> > >>>>> -----Original Message-----
> > >>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> > >>>>> Sent: Thursday, September 5, 2019 10:50 PM
> > >>>>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh
> > >>>>> <sameh.gobriel@intel.com>; Richardson, Bruce
> > >>>>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> > >>>>> <pablo.de.lara.guarch@intel.com>
> > >>>>> Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
> > >>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter
> > >>>>> subtests
> > >>>>>
> > >>>>> From: Amit Gupta <agupta3@marvell.com>
> > >>>>>
> > >>>>> hash_readwrite meson test was taking longer time to complete.
> > >>>>> The test always get TIMEOUT, hence test is split into functional
> > >>>>> and perf test. perf test is being moved under dpdk perf
> > >>>>> testsuites in meson build.
> > >>>>>
> > >>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > >>>> [Wang, Yipeng]
> > >>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> > >>>>
> > >>>>
> > >>>> Thanks for the patch!
> > >>>
> > >>> @Wang, Yipeng1, any plan on taking this patch ?
> > >>>
> > >>>
> > >>> Regards,
> > >>> Amit
V2 changes: - Remove duplicated code in the existing patch - Add lock-free option to hash read-write functional test cases - Move existing loack-free test cases to performance tests - A typo fix in the comments for lock-free extendable table feature V1 changes: - hash_readwrite and hash_readwrite lockfree meson test was taking longer time to complete. The test always get TIMEOUT. - hash readwrtie test is split into functional and perf tests and moved to dpdk fast and perf testsuite accordingly. - hash readwrite lockfree is moved to dpdk perf testsuite. Amit Gupta (1): test/meson: hash test split into shorter subtests Honnappa Nagarahalli (4): test/hash: remove duplicated test code test/hash: add lock free reader writer functional tests test/hash: move reader writer lock free tests to perf tests hash: correct lock free extendable table support app/test/Makefile | 2 +- app/test/autotest_data.py | 14 ++- app/test/meson.build | 7 +- app/test/test_hash_readwrite.c | 117 +++++++++++++----- ...ite_lf.c => test_hash_readwrite_lf_perf.c} | 7 +- lib/librte_hash/rte_hash.h | 2 - 6 files changed, 107 insertions(+), 42 deletions(-) rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%) -- 2.17.1
From: Amit Gupta <agupta3@marvell.com> hash_readwrite meson test was taking longer time to complete. The test always get TIMEOUT, hence test is split into functional and perf test. perf test is being moved under dpdk perf testsuites in meson build. Signed-off-by: Amit Gupta <agupta3@marvell.com> Acked-by: Yipeng Wang <yipeng1.wang@intel.com> --- app/test/meson.build | 3 +- app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 1 deletion(-) diff --git a/app/test/meson.build b/app/test/meson.build index 22b0cefaa..08c0ecb3f 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -233,7 +233,7 @@ fast_test_names = [ 'distributor_autotest', 'eventdev_common_autotest', 'fbarray_autotest', - 'hash_readwrite_autotest', + 'hash_readwrite_func_autotest', 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', @@ -282,6 +282,7 @@ perf_test_names = [ 'stack_perf_autotest', 'stack_lf_perf_autotest', 'rand_perf_autotest', + 'hash_readwrite_perf_autotest', ] driver_test_names = [ diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c index 615767fb6..aa55db7fe 100644 --- a/app/test/test_hash_readwrite.c +++ b/app/test/test_hash_readwrite.c @@ -605,6 +605,150 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm, return -1; } +static int +test_hash_rw_perf_main(void) +{ + /* + * Variables used to choose different tests. + * use_htm indicates if hardware transactional memory should be used. + * reader_faster indicates if the reader threads should finish earlier + * than writer threads. This is to timing either reader threads or + * writer threads for performance numbers. + */ + int use_htm, reader_faster; + unsigned int i = 0, core_id = 0; + + if (rte_lcore_count() < 3) { + printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); + return TEST_SKIPPED; + } + + RTE_LCORE_FOREACH_SLAVE(core_id) { + slave_core_ids[i] = core_id; + i++; + } + + setlocale(LC_NUMERIC, ""); + + if (rte_tm_supported()) { + printf("Hardware transactional memory (lock elision) " + "is supported\n"); + + printf("Test read-write with Hardware transactional memory\n"); + + use_htm = 1; + + reader_faster = 1; + if (test_hash_readwrite_perf(&htm_results, use_htm, + reader_faster) < 0) + return -1; + + reader_faster = 0; + if (test_hash_readwrite_perf(&htm_results, use_htm, + reader_faster) < 0) + return -1; + } else { + printf("Hardware transactional memory (lock elision) " + "is NOT supported\n"); + } + + printf("Test read-write without Hardware transactional memory\n"); + use_htm = 0; + + reader_faster = 1; + if (test_hash_readwrite_perf(&non_htm_results, use_htm, + reader_faster) < 0) + return -1; + reader_faster = 0; + if (test_hash_readwrite_perf(&non_htm_results, use_htm, + reader_faster) < 0) + return -1; + + printf("================\n"); + printf("Results summary:\n"); + printf("================\n"); + + printf("single read: %u\n", htm_results.single_read); + printf("single write: %u\n", htm_results.single_write); + for (i = 0; i < NUM_TEST; i++) { + printf("+++ core_cnt: %u +++\n", core_cnt[i]); + printf("HTM:\n"); + printf(" read only: %u\n", htm_results.read_only[i]); + printf(" write only: %u\n", htm_results.write_only[i]); + printf(" read-write read: %u\n", htm_results.read_write_r[i]); + printf(" read-write write: %u\n", htm_results.read_write_w[i]); + + printf("non HTM:\n"); + printf(" read only: %u\n", non_htm_results.read_only[i]); + printf(" write only: %u\n", non_htm_results.write_only[i]); + printf(" read-write read: %u\n", + non_htm_results.read_write_r[i]); + printf(" read-write write: %u\n", + non_htm_results.read_write_w[i]); + } + + return 0; +} + +static int +test_hash_rw_func_main(void) +{ + /* + * Variables used to choose different tests. + * use_htm indicates if hardware transactional memory should be used. + * reader_faster indicates if the reader threads should finish earlier + * than writer threads. This is to timing either reader threads or + * writer threads for performance numbers. + */ + int use_htm, use_ext; + unsigned int i = 0, core_id = 0; + + if (rte_lcore_count() < 3) { + printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); + return TEST_SKIPPED; + } + + RTE_LCORE_FOREACH_SLAVE(core_id) { + slave_core_ids[i] = core_id; + i++; + } + + setlocale(LC_NUMERIC, ""); + + if (rte_tm_supported()) { + printf("Hardware transactional memory (lock elision) " + "is supported\n"); + + printf("Test read-write with Hardware transactional memory\n"); + + use_htm = 1; + use_ext = 0; + + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + use_ext = 1; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + } else { + printf("Hardware transactional memory (lock elision) " + "is NOT supported\n"); + } + + printf("Test read-write without Hardware transactional memory\n"); + use_htm = 0; + use_ext = 0; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + use_ext = 1; + if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + return -1; + + return 0; +} + static int test_hash_readwrite_main(void) { @@ -706,3 +850,5 @@ test_hash_readwrite_main(void) } REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main); +REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main); +REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main); -- 2.17.1
The test case target 'hash_readwrite_autotest' is covered by 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'. Hence, it is removed along with its test code. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> --- app/test/autotest_data.py | 10 +++- app/test/test_hash_readwrite.c | 101 --------------------------------- 2 files changed, 8 insertions(+), 103 deletions(-) diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py index 6deb97bcc..71db4b3f6 100644 --- a/app/test/autotest_data.py +++ b/app/test/autotest_data.py @@ -664,8 +664,14 @@ "Report": None, }, { - "Name": "Hash read-write concurrency autotest", - "Command": "hash_readwrite_autotest", + "Name": "Hash read-write concurrency functional autotest", + "Command": "hash_readwrite_func_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash read-write concurrency perf autotest", + "Command": "hash_readwrite_perf_autotest", "Func": default_autotest, "Report": None, }, diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c index aa55db7fe..635ed5a9f 100644 --- a/app/test/test_hash_readwrite.c +++ b/app/test/test_hash_readwrite.c @@ -749,106 +749,5 @@ test_hash_rw_func_main(void) return 0; } -static int -test_hash_readwrite_main(void) -{ - /* - * Variables used to choose different tests. - * use_htm indicates if hardware transactional memory should be used. - * reader_faster indicates if the reader threads should finish earlier - * than writer threads. This is to timing either reader threads or - * writer threads for performance numbers. - */ - int use_htm, use_ext, reader_faster; - unsigned int i = 0, core_id = 0; - - if (rte_lcore_count() < 3) { - printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); - return TEST_SKIPPED; - } - - RTE_LCORE_FOREACH_SLAVE(core_id) { - slave_core_ids[i] = core_id; - i++; - } - - setlocale(LC_NUMERIC, ""); - - if (rte_tm_supported()) { - printf("Hardware transactional memory (lock elision) " - "is supported\n"); - - printf("Test read-write with Hardware transactional memory\n"); - - use_htm = 1; - use_ext = 0; - - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) - return -1; - - use_ext = 1; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) - return -1; - - reader_faster = 1; - if (test_hash_readwrite_perf(&htm_results, use_htm, - reader_faster) < 0) - return -1; - - reader_faster = 0; - if (test_hash_readwrite_perf(&htm_results, use_htm, - reader_faster) < 0) - return -1; - } else { - printf("Hardware transactional memory (lock elision) " - "is NOT supported\n"); - } - - printf("Test read-write without Hardware transactional memory\n"); - use_htm = 0; - use_ext = 0; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) - return -1; - - use_ext = 1; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) - return -1; - - reader_faster = 1; - if (test_hash_readwrite_perf(&non_htm_results, use_htm, - reader_faster) < 0) - return -1; - reader_faster = 0; - if (test_hash_readwrite_perf(&non_htm_results, use_htm, - reader_faster) < 0) - return -1; - - printf("================\n"); - printf("Results summary:\n"); - printf("================\n"); - - printf("single read: %u\n", htm_results.single_read); - printf("single write: %u\n", htm_results.single_write); - for (i = 0; i < NUM_TEST; i++) { - printf("+++ core_cnt: %u +++\n", core_cnt[i]); - printf("HTM:\n"); - printf(" read only: %u\n", htm_results.read_only[i]); - printf(" write only: %u\n", htm_results.write_only[i]); - printf(" read-write read: %u\n", htm_results.read_write_r[i]); - printf(" read-write write: %u\n", htm_results.read_write_w[i]); - - printf("non HTM:\n"); - printf(" read only: %u\n", non_htm_results.read_only[i]); - printf(" write only: %u\n", non_htm_results.write_only[i]); - printf(" read-write read: %u\n", - non_htm_results.read_write_r[i]); - printf(" read-write write: %u\n", - non_htm_results.read_write_w[i]); - } - - return 0; -} - -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main); REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main); REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main); -- 2.17.1
Add lock-free reader writer concurrency functional tests. These tests will provide the same coverage that non lock-free APIs have. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> --- app/test/test_hash_readwrite.c | 58 +++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644 --- a/app/test/test_hash_readwrite.c +++ b/app/test/test_hash_readwrite.c @@ -121,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg) } static int -init_params(int use_ext, int use_htm, int use_jhash) +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash) { unsigned int i; @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int use_jhash) else hash_params.hash_func = rte_hash_crc; + hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; if (use_htm) - hash_params.extra_flag = - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; + hash_params.extra_flag |= + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT; + if (rw_lf) + hash_params.extra_flag |= + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF; else - hash_params.extra_flag = - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; + hash_params.extra_flag |= + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY; if (use_ext) hash_params.extra_flag |= @@ -195,7 +196,7 @@ init_params(int use_ext, int use_htm, int use_jhash) } static int -test_hash_readwrite_functional(int use_ext, int use_htm) +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int use_ext) { unsigned int i; const void *next_key; @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int use_htm) rte_atomic64_init(&ginsertions); rte_atomic64_clear(&ginsertions); - if (init_params(use_ext, use_htm, use_jhash) != 0) + if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0) goto err; if (use_ext) @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int use_htm) tbl_rw_test_param.num_insert * slave_cnt; + printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n", + use_htm, use_rw_lf, use_ext); printf("++++++++Start function tests:+++++++++\n"); /* Fire all threads. */ @@ -379,7 +382,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm, rte_atomic64_init(&gwrite_cycles); rte_atomic64_clear(&gwrite_cycles); - if (init_params(0, use_htm, use_jhash) != 0) + if (init_params(0, use_htm, 0, use_jhash) != 0) goto err; /* @@ -700,7 +703,6 @@ test_hash_rw_func_main(void) * than writer threads. This is to timing either reader threads or * writer threads for performance numbers. */ - int use_htm, use_ext; unsigned int i = 0, core_id = 0; if (rte_lcore_count() < 3) { @@ -721,29 +723,41 @@ test_hash_rw_func_main(void) printf("Test read-write with Hardware transactional memory\n"); - use_htm = 1; - use_ext = 0; + /* htm = 1, rw_lf = 0, ext = 0 */ + if (test_hash_readwrite_functional(1, 0, 0) < 0) + return -1; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + /* htm = 1, rw_lf = 1, ext = 0 */ + if (test_hash_readwrite_functional(1, 1, 0) < 0) return -1; - use_ext = 1; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + /* htm = 1, rw_lf = 0, ext = 1 */ + if (test_hash_readwrite_functional(1, 0, 1) < 0) return -1; + /* htm = 1, rw_lf = 1, ext = 1 */ + if (test_hash_readwrite_functional(1, 1, 1) < 0) + return -1; } else { printf("Hardware transactional memory (lock elision) " "is NOT supported\n"); } printf("Test read-write without Hardware transactional memory\n"); - use_htm = 0; - use_ext = 0; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + /* htm = 0, rw_lf = 0, ext = 0 */ + if (test_hash_readwrite_functional(0, 0, 0) < 0) + return -1; + + /* htm = 0, rw_lf = 1, ext = 0 */ + if (test_hash_readwrite_functional(0, 1, 0) < 0) + return -1; + + /* htm = 0, rw_lf = 0, ext = 1 */ + if (test_hash_readwrite_functional(0, 0, 1) < 0) return -1; - use_ext = 1; - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) + /* htm = 0, rw_lf = 1, ext = 1 */ + if (test_hash_readwrite_functional(0, 1, 1) < 0) return -1; return 0; -- 2.17.1
Move reader writer lock free tests to performance tests. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> --- app/test/Makefile | 2 +- app/test/autotest_data.py | 4 ++-- app/test/meson.build | 4 ++-- ...t_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} | 7 ++++--- 4 files changed, 9 insertions(+), 8 deletions(-) rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%) diff --git a/app/test/Makefile b/app/test/Makefile index 57930c00b..d955dbb03 100644 --- a/app/test/Makefile +++ b/app/test/Makefile @@ -122,7 +122,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_perf.c SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c -SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c +SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf_perf.c SRCS-$(CONFIG_RTE_LIBRTE_RIB) += test_rib.c SRCS-$(CONFIG_RTE_LIBRTE_RIB) += test_rib6.c diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py index 71db4b3f6..7b1d01389 100644 --- a/app/test/autotest_data.py +++ b/app/test/autotest_data.py @@ -676,8 +676,8 @@ "Report": None, }, { - "Name": "Hash read-write lock-free concurrency autotest", - "Command": "hash_readwrite_lf_autotest", + "Name": "Hash read-write lock-free concurrency perf autotest", + "Command": "hash_readwrite_lf_perf_autotest", "Func": default_autotest, "Report": None, }, diff --git a/app/test/meson.build b/app/test/meson.build index 08c0ecb3f..07cf675a4 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -59,7 +59,7 @@ test_sources = files('commands.c', 'test_hash_multiwriter.c', 'test_hash_readwrite.c', 'test_hash_perf.c', - 'test_hash_readwrite_lf.c', + 'test_hash_readwrite_lf_perf.c', 'test_interrupts.c', 'test_ipsec.c', 'test_ipsec_sad.c', @@ -234,7 +234,6 @@ fast_test_names = [ 'eventdev_common_autotest', 'fbarray_autotest', 'hash_readwrite_func_autotest', - 'hash_readwrite_lf_autotest', 'ipsec_autotest', 'kni_autotest', 'kvargs_autotest', @@ -283,6 +282,7 @@ perf_test_names = [ 'stack_lf_perf_autotest', 'rand_perf_autotest', 'hash_readwrite_perf_autotest', + 'hash_readwrite_lf_perf_autotest' ] driver_test_names = [ diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf_perf.c similarity index 99% rename from app/test/test_hash_readwrite_lf.c rename to app/test/test_hash_readwrite_lf_perf.c index 97c304054..a7547bea0 100644 --- a/app/test/test_hash_readwrite_lf.c +++ b/app/test/test_hash_readwrite_lf_perf.c @@ -1241,7 +1241,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results, } static int -test_hash_readwrite_lf_main(void) +test_hash_readwrite_lf_perf_main(void) { /* * Variables used to choose different tests. @@ -1254,7 +1254,7 @@ test_hash_readwrite_lf_main(void) int ext_bkt = 0; if (rte_lcore_count() < 2) { - printf("Not enough cores for hash_readwrite_lf_autotest, expecting at least 2\n"); + printf("Not enough cores for hash_readwrite_lf_perf_autotest, expecting at least 2\n"); return TEST_SKIPPED; } @@ -1431,4 +1431,5 @@ test_hash_readwrite_lf_main(void) return 0; } -REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main); +REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest, + test_hash_readwrite_lf_perf_main); -- 2.17.1
Lock-free extendable table is supported. Correct the comments. Fixes: f401363d984a ("hash: support lock-free extendable bucket") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> --- lib/librte_hash/rte_hash.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 063701173..ed0673b73 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -51,8 +51,6 @@ extern "C" { /** Flag to support lock free reader writer concurrency. Both single writer * and multi writer use cases are supported. - * Currently, extendable bucket table feature is not supported with - * this feature. */ #define RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF 0x20 -- 2.17.1
On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > > The test case target 'hash_readwrite_autotest' is covered by > 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'. > Hence, it is removed along with its test code. > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > --- > app/test/autotest_data.py | 10 +++- > app/test/test_hash_readwrite.c | 101 --------------------------------- > 2 files changed, 8 insertions(+), 103 deletions(-) > > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py > index 6deb97bcc..71db4b3f6 100644 > --- a/app/test/autotest_data.py > +++ b/app/test/autotest_data.py > @@ -664,8 +664,14 @@ > "Report": None, > }, > { > - "Name": "Hash read-write concurrency autotest", > - "Command": "hash_readwrite_autotest", > + "Name": "Hash read-write concurrency functional autotest", > + "Command": "hash_readwrite_func_autotest", > + "Func": default_autotest, > + "Report": None, > + }, > + { > + "Name": "Hash read-write concurrency perf autotest", > + "Command": "hash_readwrite_perf_autotest", > "Func": default_autotest, > "Report": None, > }, > diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c > index aa55db7fe..635ed5a9f 100644 > --- a/app/test/test_hash_readwrite.c > +++ b/app/test/test_hash_readwrite.c > @@ -749,106 +749,5 @@ test_hash_rw_func_main(void) > return 0; > } > > -static int > -test_hash_readwrite_main(void) > -{ > - /* > - * Variables used to choose different tests. > - * use_htm indicates if hardware transactional memory should be used. > - * reader_faster indicates if the reader threads should finish earlier > - * than writer threads. This is to timing either reader threads or > - * writer threads for performance numbers. > - */ > - int use_htm, use_ext, reader_faster; > - unsigned int i = 0, core_id = 0; > - > - if (rte_lcore_count() < 3) { > - printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n"); > - return TEST_SKIPPED; > - } > - > - RTE_LCORE_FOREACH_SLAVE(core_id) { > - slave_core_ids[i] = core_id; > - i++; > - } > - > - setlocale(LC_NUMERIC, ""); > - > - if (rte_tm_supported()) { > - printf("Hardware transactional memory (lock elision) " > - "is supported\n"); > - > - printf("Test read-write with Hardware transactional memory\n"); > - > - use_htm = 1; > - use_ext = 0; > - > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > - return -1; > - > - use_ext = 1; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > - return -1; > - > - reader_faster = 1; > - if (test_hash_readwrite_perf(&htm_results, use_htm, > - reader_faster) < 0) > - return -1; > - > - reader_faster = 0; > - if (test_hash_readwrite_perf(&htm_results, use_htm, > - reader_faster) < 0) > - return -1; > - } else { > - printf("Hardware transactional memory (lock elision) " > - "is NOT supported\n"); > - } > - > - printf("Test read-write without Hardware transactional memory\n"); > - use_htm = 0; > - use_ext = 0; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > - return -1; > - > - use_ext = 1; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > - return -1; > - > - reader_faster = 1; > - if (test_hash_readwrite_perf(&non_htm_results, use_htm, > - reader_faster) < 0) > - return -1; > - reader_faster = 0; > - if (test_hash_readwrite_perf(&non_htm_results, use_htm, > - reader_faster) < 0) > - return -1; > - > - printf("================\n"); > - printf("Results summary:\n"); > - printf("================\n"); > - > - printf("single read: %u\n", htm_results.single_read); > - printf("single write: %u\n", htm_results.single_write); > - for (i = 0; i < NUM_TEST; i++) { > - printf("+++ core_cnt: %u +++\n", core_cnt[i]); > - printf("HTM:\n"); > - printf(" read only: %u\n", htm_results.read_only[i]); > - printf(" write only: %u\n", htm_results.write_only[i]); > - printf(" read-write read: %u\n", htm_results.read_write_r[i]); > - printf(" read-write write: %u\n", htm_results.read_write_w[i]); > - > - printf("non HTM:\n"); > - printf(" read only: %u\n", non_htm_results.read_only[i]); > - printf(" write only: %u\n", non_htm_results.write_only[i]); > - printf(" read-write read: %u\n", > - non_htm_results.read_write_r[i]); > - printf(" read-write write: %u\n", > - non_htm_results.read_write_w[i]); > - } > - > - return 0; > -} > - > -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main); > REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main); > REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main); > -- > 2.17.1 > I can see that DTS references this test: https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164 Travis (and UNH) also runs those unit tests (via the list included in meson). Is there a reason to keep those in DTS? Thanks. -- David Marchand
On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > > Add lock-free reader writer concurrency functional tests. > These tests will provide the same coverage that non lock-free > APIs have. > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > --- > app/test/test_hash_readwrite.c | 58 +++++++++++++++++++++------------- > 1 file changed, 36 insertions(+), 22 deletions(-) > > diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c > index 635ed5a9f..a9429091c 100644 > --- a/app/test/test_hash_readwrite.c > +++ b/app/test/test_hash_readwrite.c > @@ -121,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg) > } > > static int > -init_params(int use_ext, int use_htm, int use_jhash) > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash) > { > unsigned int i; > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int use_jhash) > else > hash_params.hash_func = rte_hash_crc; > > + hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > if (use_htm) > - hash_params.extra_flag = > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > + hash_params.extra_flag |= > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT; > + if (rw_lf) > + hash_params.extra_flag |= > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF; > else > - hash_params.extra_flag = > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > + hash_params.extra_flag |= > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY; > > if (use_ext) > hash_params.extra_flag |= > @@ -195,7 +196,7 @@ init_params(int use_ext, int use_htm, int use_jhash) > } > > static int > -test_hash_readwrite_functional(int use_ext, int use_htm) > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int use_ext) This is a bit hard to read, please keep the same order than init_params. > { > unsigned int i; > const void *next_key; > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int use_htm) > rte_atomic64_init(&ginsertions); > rte_atomic64_clear(&ginsertions); > > - if (init_params(use_ext, use_htm, use_jhash) != 0) > + if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0) > goto err; > > if (use_ext) > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int use_htm) > tbl_rw_test_param.num_insert > * slave_cnt; > > + printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n", > + use_htm, use_rw_lf, use_ext); > printf("++++++++Start function tests:+++++++++\n"); > > /* Fire all threads. */ > @@ -379,7 +382,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm, > rte_atomic64_init(&gwrite_cycles); > rte_atomic64_clear(&gwrite_cycles); > > - if (init_params(0, use_htm, use_jhash) != 0) > + if (init_params(0, use_htm, 0, use_jhash) != 0) > goto err; > > /* > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void) > * than writer threads. This is to timing either reader threads or > * writer threads for performance numbers. > */ > - int use_htm, use_ext; The comments block just before is out of sync. > unsigned int i = 0, core_id = 0; > > if (rte_lcore_count() < 3) { > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void) > > printf("Test read-write with Hardware transactional memory\n"); > > - use_htm = 1; > - use_ext = 0; > + /* htm = 1, rw_lf = 0, ext = 0 */ I didn't like those local variables. But comments tend to get out of sync fairly easily, please remove too. > + if (test_hash_readwrite_functional(1, 0, 0) < 0) > + return -1; > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > + /* htm = 1, rw_lf = 1, ext = 0 */ > + if (test_hash_readwrite_functional(1, 1, 0) < 0) > return -1; > > - use_ext = 1; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > + /* htm = 1, rw_lf = 0, ext = 1 */ > + if (test_hash_readwrite_functional(1, 0, 1) < 0) > return -1; > > + /* htm = 1, rw_lf = 1, ext = 1 */ > + if (test_hash_readwrite_functional(1, 1, 1) < 0) > + return -1; > } else { > printf("Hardware transactional memory (lock elision) " > "is NOT supported\n"); > } > > printf("Test read-write without Hardware transactional memory\n"); > - use_htm = 0; > - use_ext = 0; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > + /* htm = 0, rw_lf = 0, ext = 0 */ > + if (test_hash_readwrite_functional(0, 0, 0) < 0) > + return -1; > + > + /* htm = 0, rw_lf = 1, ext = 0 */ > + if (test_hash_readwrite_functional(0, 1, 0) < 0) > + return -1; > + > + /* htm = 0, rw_lf = 0, ext = 1 */ > + if (test_hash_readwrite_functional(0, 0, 1) < 0) > return -1; > > - use_ext = 1; > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > + /* htm = 0, rw_lf = 1, ext = 1 */ > + if (test_hash_readwrite_functional(0, 1, 1) < 0) > return -1; > > return 0; > -- > 2.17.1 > -- David Marchand
> > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli > <honnappa.nagarahalli@arm.com> wrote: > > > > Add lock-free reader writer concurrency functional tests. > > These tests will provide the same coverage that non lock-free APIs > > have. > > > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > --- > > app/test/test_hash_readwrite.c | 58 > > +++++++++++++++++++++------------- > > 1 file changed, 36 insertions(+), 22 deletions(-) > > > > diff --git a/app/test/test_hash_readwrite.c > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644 > > --- a/app/test/test_hash_readwrite.c > > +++ b/app/test/test_hash_readwrite.c > > @@ -121,7 +121,7 @@ > test_hash_readwrite_worker(__attribute__((unused)) > > void *arg) } > > > > static int > > -init_params(int use_ext, int use_htm, int use_jhash) > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash) > > { > > unsigned int i; > > > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int > use_jhash) > > else > > hash_params.hash_func = rte_hash_crc; > > > > + hash_params.extra_flag = > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > if (use_htm) > > - hash_params.extra_flag = > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > + hash_params.extra_flag |= > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT; > > + if (rw_lf) > > + hash_params.extra_flag |= > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF; > > else > > - hash_params.extra_flag = > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > > + hash_params.extra_flag |= > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY; > > > > if (use_ext) > > hash_params.extra_flag |= @@ -195,7 +196,7 @@ > > init_params(int use_ext, int use_htm, int use_jhash) } > > > > static int > > -test_hash_readwrite_functional(int use_ext, int use_htm) > > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int > > +use_ext) > > This is a bit hard to read, please keep the same order than init_params. It looks like it is better to change the init_params. Otherwise, the code in test_hash_rw_func_main becomes hard to read. See the comment below. > > > > { > > unsigned int i; > > const void *next_key; > > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int > use_htm) > > rte_atomic64_init(&ginsertions); > > rte_atomic64_clear(&ginsertions); > > > > - if (init_params(use_ext, use_htm, use_jhash) != 0) > > + if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0) > > goto err; > > > > if (use_ext) > > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int > use_htm) > > tbl_rw_test_param.num_insert > > * slave_cnt; > > > > + printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n", > > + use_htm, use_rw_lf, use_ext); > > printf("++++++++Start function tests:+++++++++\n"); > > > > /* Fire all threads. */ > > @@ -379,7 +382,7 @@ test_hash_readwrite_perf(struct perf *perf_results, > int use_htm, > > rte_atomic64_init(&gwrite_cycles); > > rte_atomic64_clear(&gwrite_cycles); > > > > - if (init_params(0, use_htm, use_jhash) != 0) > > + if (init_params(0, use_htm, 0, use_jhash) != 0) > > goto err; > > > > /* > > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void) > > * than writer threads. This is to timing either reader threads or > > * writer threads for performance numbers. > > */ > > - int use_htm, use_ext; > > The comments block just before is out of sync. > > > > unsigned int i = 0, core_id = 0; > > > > if (rte_lcore_count() < 3) { > > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void) > > > > printf("Test read-write with Hardware transactional > > memory\n"); > > > > - use_htm = 1; > > - use_ext = 0; > > + /* htm = 1, rw_lf = 0, ext = 0 */ > > I didn't like those local variables. > But comments tend to get out of sync fairly easily, please remove too. > > > > + if (test_hash_readwrite_functional(1, 0, 0) < 0) > > + return -1; > > > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > + /* htm = 1, rw_lf = 1, ext = 0 */ > > + if (test_hash_readwrite_functional(1, 1, 0) < 0) > > return -1; > > > > - use_ext = 1; > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > + /* htm = 1, rw_lf = 0, ext = 1 */ > > + if (test_hash_readwrite_functional(1, 0, 1) < 0) > > return -1; > > > > + /* htm = 1, rw_lf = 1, ext = 1 */ > > + if (test_hash_readwrite_functional(1, 1, 1) < 0) > > + return -1; > > } else { > > printf("Hardware transactional memory (lock elision) " > > "is NOT supported\n"); > > } > > > > printf("Test read-write without Hardware transactional memory\n"); > > - use_htm = 0; > > - use_ext = 0; > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > + /* htm = 0, rw_lf = 0, ext = 0 */ > > + if (test_hash_readwrite_functional(0, 0, 0) < 0) > > + return -1; > > + > > + /* htm = 0, rw_lf = 1, ext = 0 */ > > + if (test_hash_readwrite_functional(0, 1, 0) < 0) > > + return -1; > > + > > + /* htm = 0, rw_lf = 0, ext = 1 */ > > + if (test_hash_readwrite_functional(0, 0, 1) < 0) > > return -1; > > > > - use_ext = 1; > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0) > > + /* htm = 0, rw_lf = 1, ext = 1 */ > > + if (test_hash_readwrite_functional(0, 1, 1) < 0) > > return -1; The ordering of bits (0-0-0, 0-1-0, 0-0-1, 0-1-1) looks better here. > > > > return 0; > > -- > > 2.17.1 > > > > > -- > David Marchand
On Wed, Feb 5, 2020 at 5:22 PM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> >
> > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
> > <honnappa.nagarahalli@arm.com> wrote:
> > >
> > > Add lock-free reader writer concurrency functional tests.
> > > These tests will provide the same coverage that non lock-free APIs
> > > have.
> > >
> > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > ---
> > > app/test/test_hash_readwrite.c | 58
> > > +++++++++++++++++++++-------------
> > > 1 file changed, 36 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/app/test/test_hash_readwrite.c
> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644
> > > --- a/app/test/test_hash_readwrite.c
> > > +++ b/app/test/test_hash_readwrite.c
> > > @@ -121,7 +121,7 @@
> > test_hash_readwrite_worker(__attribute__((unused))
> > > void *arg) }
> > >
> > > static int
> > > -init_params(int use_ext, int use_htm, int use_jhash)
> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
> > > {
> > > unsigned int i;
> > >
> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
> > use_jhash)
> > > else
> > > hash_params.hash_func = rte_hash_crc;
> > >
> > > + hash_params.extra_flag =
> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > > if (use_htm)
> > > - hash_params.extra_flag =
> > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
> > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > > + hash_params.extra_flag |=
> > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
> > > + if (rw_lf)
> > > + hash_params.extra_flag |=
> > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
> > > else
> > > - hash_params.extra_flag =
> > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > > + hash_params.extra_flag |=
> > > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY;
> > >
> > > if (use_ext)
> > > hash_params.extra_flag |= @@ -195,7 +196,7 @@
> > > init_params(int use_ext, int use_htm, int use_jhash) }
> > >
> > > static int
> > > -test_hash_readwrite_functional(int use_ext, int use_htm)
> > > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int
> > > +use_ext)
> >
> > This is a bit hard to read, please keep the same order than init_params.
> It looks like it is better to change the init_params. Otherwise, the code in test_hash_rw_func_main becomes hard to read. See the comment below.
>
> >
> >
> > > {
> > > unsigned int i;
> > > const void *next_key;
> > > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int
> > use_htm)
> > > rte_atomic64_init(&ginsertions);
> > > rte_atomic64_clear(&ginsertions);
> > >
> > > - if (init_params(use_ext, use_htm, use_jhash) != 0)
> > > + if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0)
> > > goto err;
> > >
> > > if (use_ext)
> > > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int
> > use_htm)
> > > tbl_rw_test_param.num_insert
> > > * slave_cnt;
> > >
> > > + printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n",
> > > + use_htm, use_rw_lf, use_ext);
> > > printf("++++++++Start function tests:+++++++++\n");
> > >
> > > /* Fire all threads. */
> > > @@ -379,7 +382,7 @@ test_hash_readwrite_perf(struct perf *perf_results,
> > int use_htm,
> > > rte_atomic64_init(&gwrite_cycles);
> > > rte_atomic64_clear(&gwrite_cycles);
> > >
> > > - if (init_params(0, use_htm, use_jhash) != 0)
> > > + if (init_params(0, use_htm, 0, use_jhash) != 0)
> > > goto err;
> > >
> > > /*
> > > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void)
> > > * than writer threads. This is to timing either reader threads or
> > > * writer threads for performance numbers.
> > > */
> > > - int use_htm, use_ext;
> >
> > The comments block just before is out of sync.
> >
> >
> > > unsigned int i = 0, core_id = 0;
> > >
> > > if (rte_lcore_count() < 3) {
> > > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void)
> > >
> > > printf("Test read-write with Hardware transactional
> > > memory\n");
> > >
> > > - use_htm = 1;
> > > - use_ext = 0;
> > > + /* htm = 1, rw_lf = 0, ext = 0 */
> >
> > I didn't like those local variables.
> > But comments tend to get out of sync fairly easily, please remove too.
> >
> >
> > > + if (test_hash_readwrite_functional(1, 0, 0) < 0)
> > > + return -1;
> > >
> > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > + /* htm = 1, rw_lf = 1, ext = 0 */
> > > + if (test_hash_readwrite_functional(1, 1, 0) < 0)
> > > return -1;
> > >
> > > - use_ext = 1;
> > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > + /* htm = 1, rw_lf = 0, ext = 1 */
> > > + if (test_hash_readwrite_functional(1, 0, 1) < 0)
> > > return -1;
> > >
> > > + /* htm = 1, rw_lf = 1, ext = 1 */
> > > + if (test_hash_readwrite_functional(1, 1, 1) < 0)
> > > + return -1;
> > > } else {
> > > printf("Hardware transactional memory (lock elision) "
> > > "is NOT supported\n");
> > > }
> > >
> > > printf("Test read-write without Hardware transactional memory\n");
> > > - use_htm = 0;
> > > - use_ext = 0;
> > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > + /* htm = 0, rw_lf = 0, ext = 0 */
> > > + if (test_hash_readwrite_functional(0, 0, 0) < 0)
> > > + return -1;
> > > +
> > > + /* htm = 0, rw_lf = 1, ext = 0 */
> > > + if (test_hash_readwrite_functional(0, 1, 0) < 0)
> > > + return -1;
> > > +
> > > + /* htm = 0, rw_lf = 0, ext = 1 */
> > > + if (test_hash_readwrite_functional(0, 0, 1) < 0)
> > > return -1;
> > >
> > > - use_ext = 1;
> > > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > + /* htm = 0, rw_lf = 1, ext = 1 */
> > > + if (test_hash_readwrite_functional(0, 1, 1) < 0)
> > > return -1;
> The ordering of bits (0-0-0, 0-1-0, 0-0-1, 0-1-1) looks better here.
Ok, forget my comment.
I just want to get rid of this series and we stop getting random
timeout in the CI.
I will take it as is and cleanup if I find some time later.
--
David Marchand
On Wed, Feb 5, 2020 at 9:48 AM David Marchand <david.marchand@redhat.com> wrote:
>
> On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
> <honnappa.nagarahalli@arm.com> wrote:
> >
> > The test case target 'hash_readwrite_autotest' is covered by
> > 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.
> > Hence, it is removed along with its test code.
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> > app/test/autotest_data.py | 10 +++-
> > app/test/test_hash_readwrite.c | 101 ---------------------------------
> > 2 files changed, 8 insertions(+), 103 deletions(-)
> >
> > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
> > index 6deb97bcc..71db4b3f6 100644
> > --- a/app/test/autotest_data.py
> > +++ b/app/test/autotest_data.py
> > @@ -664,8 +664,14 @@
> > "Report": None,
> > },
> > {
> > - "Name": "Hash read-write concurrency autotest",
> > - "Command": "hash_readwrite_autotest",
> > + "Name": "Hash read-write concurrency functional autotest",
> > + "Command": "hash_readwrite_func_autotest",
> > + "Func": default_autotest,
> > + "Report": None,
> > + },
> > + {
> > + "Name": "Hash read-write concurrency perf autotest",
> > + "Command": "hash_readwrite_perf_autotest",
> > "Func": default_autotest,
> > "Report": None,
> > },
> > diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> > index aa55db7fe..635ed5a9f 100644
> > --- a/app/test/test_hash_readwrite.c
> > +++ b/app/test/test_hash_readwrite.c
> > @@ -749,106 +749,5 @@ test_hash_rw_func_main(void)
> > return 0;
> > }
> >
> > -static int
> > -test_hash_readwrite_main(void)
> > -{
> > - /*
> > - * Variables used to choose different tests.
> > - * use_htm indicates if hardware transactional memory should be used.
> > - * reader_faster indicates if the reader threads should finish earlier
> > - * than writer threads. This is to timing either reader threads or
> > - * writer threads for performance numbers.
> > - */
> > - int use_htm, use_ext, reader_faster;
> > - unsigned int i = 0, core_id = 0;
> > -
> > - if (rte_lcore_count() < 3) {
> > - printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
> > - return TEST_SKIPPED;
> > - }
> > -
> > - RTE_LCORE_FOREACH_SLAVE(core_id) {
> > - slave_core_ids[i] = core_id;
> > - i++;
> > - }
> > -
> > - setlocale(LC_NUMERIC, "");
> > -
> > - if (rte_tm_supported()) {
> > - printf("Hardware transactional memory (lock elision) "
> > - "is supported\n");
> > -
> > - printf("Test read-write with Hardware transactional memory\n");
> > -
> > - use_htm = 1;
> > - use_ext = 0;
> > -
> > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > - return -1;
> > -
> > - use_ext = 1;
> > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > - return -1;
> > -
> > - reader_faster = 1;
> > - if (test_hash_readwrite_perf(&htm_results, use_htm,
> > - reader_faster) < 0)
> > - return -1;
> > -
> > - reader_faster = 0;
> > - if (test_hash_readwrite_perf(&htm_results, use_htm,
> > - reader_faster) < 0)
> > - return -1;
> > - } else {
> > - printf("Hardware transactional memory (lock elision) "
> > - "is NOT supported\n");
> > - }
> > -
> > - printf("Test read-write without Hardware transactional memory\n");
> > - use_htm = 0;
> > - use_ext = 0;
> > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > - return -1;
> > -
> > - use_ext = 1;
> > - if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > - return -1;
> > -
> > - reader_faster = 1;
> > - if (test_hash_readwrite_perf(&non_htm_results, use_htm,
> > - reader_faster) < 0)
> > - return -1;
> > - reader_faster = 0;
> > - if (test_hash_readwrite_perf(&non_htm_results, use_htm,
> > - reader_faster) < 0)
> > - return -1;
> > -
> > - printf("================\n");
> > - printf("Results summary:\n");
> > - printf("================\n");
> > -
> > - printf("single read: %u\n", htm_results.single_read);
> > - printf("single write: %u\n", htm_results.single_write);
> > - for (i = 0; i < NUM_TEST; i++) {
> > - printf("+++ core_cnt: %u +++\n", core_cnt[i]);
> > - printf("HTM:\n");
> > - printf(" read only: %u\n", htm_results.read_only[i]);
> > - printf(" write only: %u\n", htm_results.write_only[i]);
> > - printf(" read-write read: %u\n", htm_results.read_write_r[i]);
> > - printf(" read-write write: %u\n", htm_results.read_write_w[i]);
> > -
> > - printf("non HTM:\n");
> > - printf(" read only: %u\n", non_htm_results.read_only[i]);
> > - printf(" write only: %u\n", non_htm_results.write_only[i]);
> > - printf(" read-write read: %u\n",
> > - non_htm_results.read_write_r[i]);
> > - printf(" read-write write: %u\n",
> > - non_htm_results.read_write_w[i]);
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
> > REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
> > REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
> > --
> > 2.17.1
> >
>
> I can see that DTS references this test:
> https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164
>
> Travis (and UNH) also runs those unit tests (via the list included in meson).
>
> Is there a reason to keep those in DTS?
I will take this as a no and go ahead with this patch.
--
David Marchand
On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>
> V2 changes:
> - Remove duplicated code in the existing patch
> - Add lock-free option to hash read-write functional test cases
> - Move existing loack-free test cases to performance tests
> - A typo fix in the comments for lock-free extendable table feature
>
> V1 changes:
> - hash_readwrite and hash_readwrite lockfree meson test was
> taking longer time to complete. The test always get TIMEOUT.
> - hash readwrtie test is split into functional and perf tests
> and moved to dpdk fast and perf testsuite accordingly.
> - hash readwrite lockfree is moved to dpdk perf testsuite.
>
> Amit Gupta (1):
> test/meson: hash test split into shorter subtests
>
> Honnappa Nagarahalli (4):
> test/hash: remove duplicated test code
> test/hash: add lock free reader writer functional tests
> test/hash: move reader writer lock free tests to perf tests
> hash: correct lock free extendable table support
>
> app/test/Makefile | 2 +-
> app/test/autotest_data.py | 14 ++-
> app/test/meson.build | 7 +-
> app/test/test_hash_readwrite.c | 117 +++++++++++++-----
> ...ite_lf.c => test_hash_readwrite_lf_perf.c} | 7 +-
> lib/librte_hash/rte_hash.h | 2 -
> 6 files changed, 107 insertions(+), 42 deletions(-)
> rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%)
Squashed patch 2 in patch 1.
Series applied, thanks.
--
David Marchand
>-----Original Message-----
>From: David Marchand [mailto:david.marchand@redhat.com]
>Sent: Wednesday, February 5, 2020 8:42 AM
>To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>Cc: Amit Gupta <agupta3@marvell.com>; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
>thomas@monjalon.net; dev <dev@dpdk.org>; nd <nd@arm.com>
>Subject: Re: [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
>
>On Wed, Feb 5, 2020 at 5:22 PM Honnappa Nagarahalli
><Honnappa.Nagarahalli@arm.com> wrote:
>>
>> >
>> > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
>> > <honnappa.nagarahalli@arm.com> wrote:
>> > >
>> > > Add lock-free reader writer concurrency functional tests.
>> > > These tests will provide the same coverage that non lock-free APIs
>> > > have.
>> > >
>> > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> > > ---
>> > > app/test/test_hash_readwrite.c | 58
>> > > +++++++++++++++++++++-------------
>> > > 1 file changed, 36 insertions(+), 22 deletions(-)
>> > >
>> > > diff --git a/app/test/test_hash_readwrite.c
>> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644
>> > > --- a/app/test/test_hash_readwrite.c
>> > > +++ b/app/test/test_hash_readwrite.c
>> > > @@ -121,7 +121,7 @@
>> > test_hash_readwrite_worker(__attribute__((unused))
>> > > void *arg) }
>> > >
>> > > static int
>> > > -init_params(int use_ext, int use_htm, int use_jhash)
>> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
>> > > {
>> > > unsigned int i;
>> > >
>> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
>> > use_jhash)
>> > > else
>> > > hash_params.hash_func = rte_hash_crc;
>> > >
>> > > + hash_params.extra_flag =
>> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> > > if (use_htm)
>> > > - hash_params.extra_flag =
>> > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
>> > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>> > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> > > + hash_params.extra_flag |=
>> > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
[Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY
Flag even with HTM.
Other commits in this series look good to me and seems David already applied.
Thanks!
<snip> > >> > > > >> > > Add lock-free reader writer concurrency functional tests. > >> > > These tests will provide the same coverage that non lock-free > >> > > APIs have. > >> > > > >> > > Signed-off-by: Honnappa Nagarahalli > >> > > <honnappa.nagarahalli@arm.com> > >> > > --- > >> > > app/test/test_hash_readwrite.c | 58 > >> > > +++++++++++++++++++++------------- > >> > > 1 file changed, 36 insertions(+), 22 deletions(-) > >> > > > >> > > diff --git a/app/test/test_hash_readwrite.c > >> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c > >> > > 100644 > >> > > --- a/app/test/test_hash_readwrite.c > >> > > +++ b/app/test/test_hash_readwrite.c > >> > > @@ -121,7 +121,7 @@ > >> > test_hash_readwrite_worker(__attribute__((unused)) > >> > > void *arg) } > >> > > > >> > > static int > >> > > -init_params(int use_ext, int use_htm, int use_jhash) > >> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash) > >> > > { > >> > > unsigned int i; > >> > > > >> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int > >> > use_jhash) > >> > > else > >> > > hash_params.hash_func = rte_hash_crc; > >> > > > >> > > + hash_params.extra_flag = > >> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > >> > > if (use_htm) > >> > > - hash_params.extra_flag = > >> > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | > >> > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > >> > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > >> > > + hash_params.extra_flag |= > >> > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT; > > [Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the > RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY > Flag even with HTM. I have made RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY depend on 'rw_lf' flag. The test case HTM + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY will still run when 'rw_lf' is set to 0. > > Other commits in this series look good to me and seems David already > applied. > > Thanks!
>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>Sent: Wednesday, February 5, 2020 11:52 AM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; David Marchand <david.marchand@redhat.com>
>Cc: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; thomas@monjalon.net; dev <dev@dpdk.org>;
>nd <nd@arm.com>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
>Subject: RE: [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
>
><snip>
>
>> >> > >
>> >> > > Add lock-free reader writer concurrency functional tests.
>> >> > > These tests will provide the same coverage that non lock-free
>> >> > > APIs have.
>> >> > >
>> >> > > Signed-off-by: Honnappa Nagarahalli
>> >> > > <honnappa.nagarahalli@arm.com>
>> >> > > ---
>> >> > > app/test/test_hash_readwrite.c | 58
>> >> > > +++++++++++++++++++++-------------
>> >> > > 1 file changed, 36 insertions(+), 22 deletions(-)
>> >> > >
>> >> > > diff --git a/app/test/test_hash_readwrite.c
>> >> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c
>> >> > > 100644
>> >> > > --- a/app/test/test_hash_readwrite.c
>> >> > > +++ b/app/test/test_hash_readwrite.c
>> >> > > @@ -121,7 +121,7 @@
>> >> > test_hash_readwrite_worker(__attribute__((unused))
>> >> > > void *arg) }
>> >> > >
>> >> > > static int
>> >> > > -init_params(int use_ext, int use_htm, int use_jhash)
>> >> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
>> >> > > {
>> >> > > unsigned int i;
>> >> > >
>> >> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
>> >> > use_jhash)
>> >> > > else
>> >> > > hash_params.hash_func = rte_hash_crc;
>> >> > >
>> >> > > + hash_params.extra_flag =
>> >> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> >> > > if (use_htm)
>> >> > > - hash_params.extra_flag =
>> >> > > - RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
>> >> > > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>> >> > > - RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> >> > > + hash_params.extra_flag |=
>> >> > > + RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
>>
>> [Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the
>> RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY
>> Flag even with HTM.
>I have made RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY depend on 'rw_lf' flag. The test case HTM +
>RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY will still run when 'rw_lf' is set to 0.
>
[Wang, Yipeng]
I see, thought was an "else if". It is correct then,
Thanks!