* [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
@ 2018-11-29 18:38 Dharmik Thakkar
2019-01-11 17:10 ` Ferruh Yigit
2019-01-14 9:23 ` [dpdk-stable] [PATCH v2] " Dharmik Thakkar
0 siblings, 2 replies; 6+ messages in thread
From: Dharmik Thakkar @ 2018-11-29 18:38 UTC (permalink / raw)
To: Bruce Richardson, Pablo de Lara; +Cc: dev, Dharmik Thakkar, stable
Reset 'iter' and 'tbl_rw_test_param.found' on each iteration
to give correct result for lost and duplicated keys.
Fixes: 0eb3726ebcf14 ("test/hash: add test for read/write concurrency")
Cc: stable@dpdk.org
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---
test/test/test_hash_readwrite.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 6b695ce6e444..be93a2ebd270 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -361,7 +361,6 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
const void *next_key;
void *next_data;
- uint32_t iter = 0;
int use_jhash = 0;
uint32_t duplicated_keys = 0;
@@ -536,6 +535,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
rte_eal_mp_wait_lcore();
+ uint32_t iter = 0;
+ memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
while (rte_hash_iterate(tbl_rw_test_param.h,
&next_key, &next_data, &iter) >= 0) {
/* Search for the key in the list of keys added .*/
@@ -619,7 +620,7 @@ test_hash_readwrite_main(void)
if (rte_lcore_count() <= 2) {
printf("More than two lcores are required "
"to do read write test\n");
- return 0;
+ return -1;
}
RTE_LCORE_FOREACH_SLAVE(core_id) {
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
2018-11-29 18:38 [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test Dharmik Thakkar
@ 2019-01-11 17:10 ` Ferruh Yigit
2019-01-11 18:37 ` Wang, Yipeng1
2019-01-14 9:23 ` [dpdk-stable] [PATCH v2] " Dharmik Thakkar
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2019-01-11 17:10 UTC (permalink / raw)
To: Dharmik Thakkar, Bruce Richardson, Pablo de Lara; +Cc: dev, stable, Yipeng Wang
On 11/29/2018 6:38 PM, Dharmik Thakkar wrote:
> Reset 'iter' and 'tbl_rw_test_param.found' on each iteration
> to give correct result for lost and duplicated keys.
>
> Fixes: 0eb3726ebcf14 ("test/hash: add test for read/write concurrency")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
> test/test/test_hash_readwrite.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
> index 6b695ce6e444..be93a2ebd270 100644
> --- a/test/test/test_hash_readwrite.c
> +++ b/test/test/test_hash_readwrite.c
> @@ -361,7 +361,6 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
>
> const void *next_key;
> void *next_data;
> - uint32_t iter = 0;
> int use_jhash = 0;
>
> uint32_t duplicated_keys = 0;
> @@ -536,6 +535,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
>
> rte_eal_mp_wait_lcore();
>
> + uint32_t iter = 0;
Logically looks good. Only we don't tend to declare the variables in the middle
of the scope, you may prefer to keep deceleration at its place but set 'iter' to
zero here.
> + memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
> while (rte_hash_iterate(tbl_rw_test_param.h,
> &next_key, &next_data, &iter) >= 0) {
> /* Search for the key in the list of keys added .*/
> @@ -619,7 +620,7 @@ test_hash_readwrite_main(void)
> if (rte_lcore_count() <= 2) {
> printf("More than two lcores are required "
> "to do read write test\n");
> - return 0;
> + return -1;
This is something not mentioned in the commit log, changes the default return
value of test when not enough resources provided, cc'ed Yipeng for comment.
If decided to keep this change, please update commit log to mention from it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
2019-01-11 17:10 ` Ferruh Yigit
@ 2019-01-11 18:37 ` Wang, Yipeng1
2019-01-14 7:12 ` Dharmik Thakkar
0 siblings, 1 reply; 6+ messages in thread
From: Wang, Yipeng1 @ 2019-01-11 18:37 UTC (permalink / raw)
To: Yigit, Ferruh, Dharmik Thakkar, Richardson, Bruce, De Lara Guarch, Pablo
Cc: dev, stable
Thanks for the bug fix! Nice catch!
>-----Original Message-----
>From: Yigit, Ferruh
>Sent: Friday, January 11, 2019 9:10 AM
>To: Dharmik Thakkar <dharmik.thakkar@arm.com>; Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
><pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; stable@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>
>Subject: Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
>> uint32_t duplicated_keys = 0;
>> @@ -536,6 +535,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
>>
>> rte_eal_mp_wait_lcore();
>>
>> + uint32_t iter = 0;
>
>Logically looks good. Only we don't tend to declare the variables in the middle
>of the scope, you may prefer to keep deceleration at its place but set 'iter' to
>zero here.
[Wang, Yipeng] Agree.
>
>> + memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
[Wang, Yipeng] test failed because of this line I think, the found is uint32_t array, so should be TOTAL_ENTRY * 4, or just change found to be int8
>> while (rte_hash_iterate(tbl_rw_test_param.h,
>> &next_key, &next_data, &iter) >= 0) {
>> /* Search for the key in the list of keys added .*/
>> @@ -619,7 +620,7 @@ test_hash_readwrite_main(void)
>> if (rte_lcore_count() <= 2) {
>> printf("More than two lcores are required "
>> "to do read write test\n");
>> - return 0;
>> + return -1;
>
>This is something not mentioned in the commit log, changes the default return
>value of test when not enough resources provided, cc'ed Yipeng for comment.
>
>If decided to keep this change, please update commit log to mention from it.
[Wang, Yipeng] Yes it should be -1. Thanks for the fix. Please fix the commit msg as Ferruh suggested
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
2019-01-11 18:37 ` Wang, Yipeng1
@ 2019-01-14 7:12 ` Dharmik Thakkar
0 siblings, 0 replies; 6+ messages in thread
From: Dharmik Thakkar @ 2019-01-14 7:12 UTC (permalink / raw)
To: Wang, Yipeng1
Cc: Yigit, Ferruh, Richardson, Bruce, De Lara Guarch, Pablo, dev, stable, nd
Thank you Ferruh and Yipeng! I will make the recommended changes and update the version.
> On Jan 12, 2019, at 12:07 AM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
>
> Thanks for the bug fix! Nice catch!
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, January 11, 2019 9:10 AM
>> To: Dharmik Thakkar <dharmik.thakkar@arm.com>; Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; stable@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test
>>> uint32_t duplicated_keys = 0;
>>> @@ -536,6 +535,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
>>>
>>> rte_eal_mp_wait_lcore();
>>>
>>> + uint32_t iter = 0;
>>
>> Logically looks good. Only we don't tend to declare the variables in the middle
>> of the scope, you may prefer to keep deceleration at its place but set 'iter' to
>> zero here.
> [Wang, Yipeng] Agree.
>>
>>> + memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
>
> [Wang, Yipeng] test failed because of this line I think, the found is uint32_t array, so should be TOTAL_ENTRY * 4, or just change found to be int8
>
>>> while (rte_hash_iterate(tbl_rw_test_param.h,
>>> &next_key, &next_data, &iter) >= 0) {
>>> /* Search for the key in the list of keys added .*/
>>> @@ -619,7 +620,7 @@ test_hash_readwrite_main(void)
>>> if (rte_lcore_count() <= 2) {
>>> printf("More than two lcores are required "
>>> "to do read write test\n");
>>> - return 0;
>>> + return -1;
>>
>> This is something not mentioned in the commit log, changes the default return
>> value of test when not enough resources provided, cc'ed Yipeng for comment.
>>
>> If decided to keep this change, please update commit log to mention from it.
> [Wang, Yipeng] Yes it should be -1. Thanks for the fix. Please fix the commit msg as Ferruh suggested
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-stable] [PATCH v2] test/hash: reset iter and found in perf test
2018-11-29 18:38 [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test Dharmik Thakkar
2019-01-11 17:10 ` Ferruh Yigit
@ 2019-01-14 9:23 ` Dharmik Thakkar
2019-01-14 23:19 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
1 sibling, 1 reply; 6+ messages in thread
From: Dharmik Thakkar @ 2019-01-14 9:23 UTC (permalink / raw)
To: dev; +Cc: Dharmik Thakkar, stable
Reset 'iter' and 'tbl_rw_test_param.found' on each iteration
to give correct result for lost and duplicated keys.
This patch also changes the default return value of the test to -1
when not enough resources are provided.
Fixes: 0eb3726ebcf14 ("test/hash: add test for read/write concurrency")
Cc: stable@dpdk.org
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Reviewed-by: Yipeng Wang <yipeng1.wang@intel.com>
--
v2:
* Update commit message
* Change 'found' array type to uint8_t
* Restore declaration of 'iter' to the start
---
test/test/test_hash_readwrite.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 6b695ce6e444..480ae97d857d 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -39,7 +39,7 @@ static struct perf htm_results, non_htm_results;
struct {
uint32_t *keys;
- uint32_t *found;
+ uint8_t *found;
uint32_t num_insert;
uint32_t rounded_tot_insert;
struct rte_hash *h;
@@ -126,7 +126,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
unsigned int i;
uint32_t *keys = NULL;
- uint32_t *found = NULL;
+ uint8_t *found = NULL;
struct rte_hash *handle;
struct rte_hash_parameters hash_params = {
@@ -173,7 +173,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
goto err;
}
- found = rte_zmalloc(NULL, sizeof(uint32_t) * TOTAL_ENTRY, 0);
+ found = rte_zmalloc(NULL, sizeof(uint8_t) * TOTAL_ENTRY, 0);
if (found == NULL) {
printf("RTE_ZMALLOC failed\n");
goto err;
@@ -361,7 +361,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
const void *next_key;
void *next_data;
- uint32_t iter = 0;
+ uint32_t iter;
int use_jhash = 0;
uint32_t duplicated_keys = 0;
@@ -536,6 +536,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
rte_eal_mp_wait_lcore();
+ iter = 0;
+ memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
while (rte_hash_iterate(tbl_rw_test_param.h,
&next_key, &next_data, &iter) >= 0) {
/* Search for the key in the list of keys added .*/
@@ -619,7 +621,7 @@ test_hash_readwrite_main(void)
if (rte_lcore_count() <= 2) {
printf("More than two lcores are required "
"to do read write test\n");
- return 0;
+ return -1;
}
RTE_LCORE_FOREACH_SLAVE(core_id) {
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] test/hash: reset iter and found in perf test
2019-01-14 9:23 ` [dpdk-stable] [PATCH v2] " Dharmik Thakkar
@ 2019-01-14 23:19 ` Thomas Monjalon
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2019-01-14 23:19 UTC (permalink / raw)
To: Dharmik Thakkar; +Cc: dev, stable
14/01/2019 10:23, Dharmik Thakkar:
> Reset 'iter' and 'tbl_rw_test_param.found' on each iteration
> to give correct result for lost and duplicated keys.
>
> This patch also changes the default return value of the test to -1
> when not enough resources are provided.
>
> Fixes: 0eb3726ebcf14 ("test/hash: add test for read/write concurrency")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Reviewed-by: Yipeng Wang <yipeng1.wang@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-14 23:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 18:38 [dpdk-stable] [PATCH] test/hash: reset iter and found in perf test Dharmik Thakkar
2019-01-11 17:10 ` Ferruh Yigit
2019-01-11 18:37 ` Wang, Yipeng1
2019-01-14 7:12 ` Dharmik Thakkar
2019-01-14 9:23 ` [dpdk-stable] [PATCH v2] " Dharmik Thakkar
2019-01-14 23:19 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).