diff mbox series

[v2,3/7] test/hash: fix rw test with non-consecutive cores

Message ID 1537550255-252066-4-git-send-email-yipeng1.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series hash: add extendable bucket and partial key hashing |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wang, Yipeng1 Sept. 21, 2018, 5:17 p.m. UTC 
the multi-reader and multi-writer rte_hash unit test does not
work correctly with non-consicutive core ids. This commit
fixes the issue.

Fixes: 0eb3726ebcf1 ("test/hash: add test for read/write concurrency")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_readwrite.c | 78 ++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 29 deletions(-)

Comments

Bruce Richardson Sept. 26, 2018, 11:02 a.m. UTC | #1 
On Fri, Sep 21, 2018 at 10:17:31AM -0700, Yipeng Wang wrote:
> the multi-reader and multi-writer rte_hash unit test does not
> work correctly with non-consicutive core ids. This commit
> fixes the issue.
> 
> Fixes: 0eb3726ebcf1 ("test/hash: add test for read/write concurrency")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---

When testing this patch, I see that the read-write autotests are not
currently in the meson.build file for the test binary. I think this
patchset should include this fix too, as a separate patch.
Bruce Richardson Sept. 26, 2018, 11:13 a.m. UTC | #2 
On Fri, Sep 21, 2018 at 10:17:31AM -0700, Yipeng Wang wrote:
> the multi-reader and multi-writer rte_hash unit test does not
> work correctly with non-consicutive core ids. This commit
> fixes the issue.
> 
> Fixes: 0eb3726ebcf1 ("test/hash: add test for read/write concurrency")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_readwrite.c | 78 ++++++++++++++++++++++++++---------------
>  1 file changed, 49 insertions(+), 29 deletions(-)
> 

With existing code, testing with "-l 0,2,4,6,40,42,44,48" gives error:

++++++++Start function tests:+++++++++
Core #2 inserting and reading 1966080: 3,932,160 - 5,898,240
Core #4 inserting and reading 1966080: 7,864,320 - 9,830,400
Core #6 inserting and reading 1966080: 11,796,480 - 13,762,560
Core #40 inserting and reading 1966080: 78,643,200 - 80,609,280
Core #42 inserting and reading 1966080: 82,575,360 - 84,541,440
Core #44 inserting and reading 1966080: 86,507,520 - 88,473,600
Core #48 inserting and reading 1966080: 94,371,840 - 96,337,920
Core #0 inserting and reading 1966080: 0 - 1,966,080
key 1966080 is lost
1 key lost
Test Failed


With this patch applies, test runs as expected.

Tested-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Wang, Yipeng1 Sept. 27, 2018, 3:40 a.m. UTC | #3 
I added another commit for this. Please test.

Thanks!

>-----Original Message-----
>
>When testing this patch, I see that the read-write autotests are not
>currently in the meson.build file for the test binary. I think this
>patchset should include this fix too, as a separate patch.
diff mbox series

Patch

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 55ae33d..2a4f7b9 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -24,6 +24,7 @@ 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
+unsigned int slave_core_ids[RTE_MAX_LCORE];
 struct perf {
 	uint32_t single_read;
 	uint32_t single_write;
@@ -60,12 +61,15 @@  test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t begin, cycles;
 	int ret;
 
-	offset = (lcore_id - rte_get_master_lcore())
-			* tbl_rw_test_param.num_insert;
+	for (i = 0; i < rte_lcore_count(); i++) {
+		if (slave_core_ids[i] == lcore_id)
+			break;
+	}
+	offset = tbl_rw_test_param.num_insert * i;
 
 	printf("Core #%d inserting and reading %d: %'"PRId64" - %'"PRId64"\n",
 	       lcore_id, tbl_rw_test_param.num_insert,
-	       offset, offset + tbl_rw_test_param.num_insert);
+	       offset, offset + tbl_rw_test_param.num_insert - 1);
 
 	begin = rte_rdtsc_precise();
 
@@ -171,6 +175,7 @@  test_hash_readwrite_functional(int use_htm)
 	uint32_t duplicated_keys = 0;
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
+	int slave_cnt = rte_lcore_count() - 1;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -182,17 +187,17 @@  test_hash_readwrite_functional(int use_htm)
 		goto err;
 
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / rte_lcore_count();
+		TOTAL_INSERT / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
-		* rte_lcore_count();
+		* slave_cnt;
 
 	printf("++++++++Start function tests:+++++++++\n");
 
 	/* Fire all threads. */
 	rte_eal_mp_remote_launch(test_hash_readwrite_worker,
-				 NULL, CALL_MASTER);
+				 NULL, SKIP_MASTER);
 	rte_eal_mp_wait_lcore();
 
 	while (rte_hash_iterate(tbl_rw_test_param.h, &next_key,
@@ -249,7 +254,7 @@  test_hash_readwrite_functional(int use_htm)
 }
 
 static int
-test_rw_reader(__attribute__((unused)) void *arg)
+test_rw_reader(void *arg)
 {
 	uint64_t i;
 	uint64_t begin, cycles;
@@ -276,7 +281,7 @@  test_rw_reader(__attribute__((unused)) void *arg)
 }
 
 static int
-test_rw_writer(__attribute__((unused)) void *arg)
+test_rw_writer(void *arg)
 {
 	uint64_t i;
 	uint32_t lcore_id = rte_lcore_id();
@@ -285,8 +290,13 @@  test_rw_writer(__attribute__((unused)) void *arg)
 	uint64_t start_coreid = (uint64_t)(uintptr_t)arg;
 	uint64_t offset;
 
-	offset = TOTAL_INSERT / 2 + (lcore_id - start_coreid)
-					* tbl_rw_test_param.num_insert;
+	for (i = 0; i < rte_lcore_count(); i++) {
+		if (slave_core_ids[i] == lcore_id)
+			break;
+	}
+
+	offset = TOTAL_INSERT / 2 + (i - (start_coreid)) *
+				tbl_rw_test_param.num_insert;
 	begin = rte_rdtsc_precise();
 	for (i = offset; i < offset + tbl_rw_test_param.num_insert; i++) {
 		ret = rte_hash_add_key_data(tbl_rw_test_param.h,
@@ -384,8 +394,8 @@  test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 	perf_results->single_read = end / i;
 
 	for (n = 0; n < NUM_TEST; n++) {
-		unsigned int tot_lcore = rte_lcore_count();
-		if (tot_lcore < core_cnt[n] * 2 + 1)
+		unsigned int tot_slave_lcore = rte_lcore_count() - 1;
+		if (tot_slave_lcore < core_cnt[n] * 2)
 			goto finish;
 
 		rte_atomic64_clear(&greads);
@@ -415,17 +425,19 @@  test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 		 */
 
 		/* Test only reader cases */
-		for (i = 1; i <= core_cnt[n]; i++)
+		for (i = 0; i < core_cnt[n]; i++)
 			rte_eal_remote_launch(test_rw_reader,
-					(void *)(uintptr_t)read_cnt, i);
+					(void *)(uintptr_t)read_cnt,
+					slave_core_ids[i]);
 
 		rte_eal_mp_wait_lcore();
 
 		start_coreid = i;
 		/* Test only writer cases */
-		for (; i <= core_cnt[n] * 2; i++)
+		for (; i < core_cnt[n] * 2; i++)
 			rte_eal_remote_launch(test_rw_writer,
-					(void *)((uintptr_t)start_coreid), i);
+					(void *)((uintptr_t)start_coreid),
+					slave_core_ids[i]);
 
 		rte_eal_mp_wait_lcore();
 
@@ -464,22 +476,26 @@  test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 			}
 		}
 
-		start_coreid = core_cnt[n] + 1;
+		start_coreid = core_cnt[n];
 
 		if (reader_faster) {
-			for (i = core_cnt[n] + 1; i <= core_cnt[n] * 2; i++)
+			for (i = core_cnt[n]; i < core_cnt[n] * 2; i++)
 				rte_eal_remote_launch(test_rw_writer,
-					(void *)((uintptr_t)start_coreid), i);
-			for (i = 1; i <= core_cnt[n]; i++)
+					(void *)((uintptr_t)start_coreid),
+					slave_core_ids[i]);
+			for (i = 0; i < core_cnt[n]; i++)
 				rte_eal_remote_launch(test_rw_reader,
-					(void *)(uintptr_t)read_cnt, i);
+					(void *)(uintptr_t)read_cnt,
+					slave_core_ids[i]);
 		} else {
-			for (i = 1; i <= core_cnt[n]; i++)
+			for (i = 0; i < core_cnt[n]; i++)
 				rte_eal_remote_launch(test_rw_reader,
-					(void *)(uintptr_t)read_cnt, i);
-			for (; i <= core_cnt[n] * 2; i++)
+					(void *)(uintptr_t)read_cnt,
+					slave_core_ids[i]);
+			for (; i < core_cnt[n] * 2; i++)
 				rte_eal_remote_launch(test_rw_writer,
-					(void *)((uintptr_t)start_coreid), i);
+					(void *)((uintptr_t)start_coreid),
+					slave_core_ids[i]);
 		}
 
 		rte_eal_mp_wait_lcore();
@@ -562,13 +578,19 @@  test_hash_readwrite_main(void)
 	 * writer threads for performance numbers.
 	 */
 	int use_htm, reader_faster;
+	unsigned int i = 0, core_id = 0;
 
-	if (rte_lcore_count() == 1) {
-		printf("More than one lcore is required "
+	if (rte_lcore_count() <= 2) {
+		printf("More than two lcores are required "
 			"to do read write test\n");
 		return 0;
 	}
 
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		slave_core_ids[i] = core_id;
+		i++;
+	}
+
 	setlocale(LC_NUMERIC, "");
 
 	if (rte_tm_supported()) {
@@ -610,8 +632,6 @@  test_hash_readwrite_main(void)
 
 	printf("Results summary:\n");
 
-	int i;
-
 	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++) {