DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places
@ 2018-10-24 18:09 Yipeng Wang
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-24 18:09 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: improve readwrite test to consider the extendable table.
commit 4: remove scaling unit test.


V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (4):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test

 lib/librte_hash/rte_cuckoo_hash.c |   4 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |   6 --
 test/test/meson.build             |   2 -
 test/test/test_hash_multiwriter.c |  10 +-
 test/test/test_hash_readwrite.c   |  70 +++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 69 insertions(+), 215 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
@ 2018-10-24 18:09 ` Yipeng Wang
  2018-10-25  6:31   ` Mattias Rönnblom
  2018-10-26  0:24   ` Honnappa Nagarahalli
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash Yipeng Wang
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-24 18:09 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..4a2647e 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash
  2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
@ 2018-10-24 18:09 ` Yipeng Wang
  2018-10-25  9:32   ` Bruce Richardson
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table Yipeng Wang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 31+ messages in thread
From: Yipeng Wang @ 2018-10-24 18:09 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..456bc5f 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -31,6 +32,9 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+/* Use jhash or crc hash */
+#define USE_JHASH 1
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -108,10 +112,14 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
+	if (USE_JHASH)
+		hash_params.hash_func = rte_jhash;
+	else
+		hash_params.hash_func = rte_hash_crc;
+
 	if (use_htm)
 		hash_params.extra_flag =
 			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table
  2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash Yipeng Wang
@ 2018-10-24 18:09 ` Yipeng Wang
  2018-10-25  9:34   ` Bruce Richardson
  2018-10-26  0:32   ` Honnappa Nagarahalli
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test Yipeng Wang
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
  4 siblings, 2 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-24 18:09 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This commit improves the readwrite test to consider
extendable table feature and add more functional tests
to cover more corner cases.

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

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..13c28e0 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -21,6 +21,8 @@
 #define TOTAL_ENTRY (16*1024*1024)
 #define TOTAL_INSERT (15*1024*1024)
 
+#define TOTAL_INSERT_EXT (16*1024*1024)
+
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
@@ -59,8 +61,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +83,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
@@ -100,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -129,6 +150,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -167,7 +195,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -178,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -185,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -345,7 +379,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(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -579,7 +613,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -602,7 +636,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(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;
@@ -621,8 +661,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(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)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test
  2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
                   ` (2 preceding siblings ...)
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table Yipeng Wang
@ 2018-10-24 18:09 ` Yipeng Wang
  2018-10-25  9:34   ` Bruce Richardson
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
  4 siblings, 1 reply; 31+ messages in thread
From: Yipeng Wang @ 2018-10-24 18:09 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 41028cc..276e7cd 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -112,7 +112,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 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_scaling.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
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index f68d9b1..31020f1 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index e403bd5..c0b1928 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -168,7 +167,6 @@ test_names = [
 	'eventdev_sw_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
@ 2018-10-25  6:31   ` Mattias Rönnblom
  2018-10-25  9:30     ` Bruce Richardson
  2018-10-26  0:24   ` Honnappa Nagarahalli
  1 sibling, 1 reply; 31+ messages in thread
From: Mattias Rönnblom @ 2018-10-25  6:31 UTC (permalink / raw)
  To: Yipeng Wang, bruce.richardson
  Cc: stephen, dev, honnappa.nagarahalli, sameh.gobriel

On 2018-10-24 20:09, Yipeng Wang wrote:
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared
> data structure, the rte_pause is not needed.
> 

I'm guessing the <rte_pause.h> include is longer needed.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-25  6:31   ` Mattias Rönnblom
@ 2018-10-25  9:30     ` Bruce Richardson
  0 siblings, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2018-10-25  9:30 UTC (permalink / raw)
  To: Mattias Rönnblom
  Cc: Yipeng Wang, stephen, dev, honnappa.nagarahalli, sameh.gobriel

On Thu, Oct 25, 2018 at 08:31:16AM +0200, Mattias Rönnblom wrote:
> On 2018-10-24 20:09, Yipeng Wang wrote:
> > There is a rte_pause in hash table reset function.
> > Since the loop is not a polling loop on shared
> > data structure, the rte_pause is not needed.
> > 
> 
> I'm guessing the <rte_pause.h> include is longer needed.

Good point, otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash Yipeng Wang
@ 2018-10-25  9:32   ` Bruce Richardson
  0 siblings, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2018-10-25  9:32 UTC (permalink / raw)
  To: Yipeng Wang; +Cc: stephen, dev, honnappa.nagarahalli, sameh.gobriel

On Wed, Oct 24, 2018 at 11:09:28AM -0700, Yipeng Wang wrote:
> With sequential key, the test will cover more corner
> cases with jhash instead of crc hash, since jhash
> generates more random hash pattern on sequential key.
> It is useful for functional verification.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_multiwriter.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
> index 6a3eb10..456bc5f 100644
> --- a/test/test/test_hash_multiwriter.c
> +++ b/test/test/test_hash_multiwriter.c
> @@ -12,6 +12,7 @@
>  #include <rte_malloc.h>
>  #include <rte_random.h>
>  #include <rte_spinlock.h>
> +#include <rte_jhash.h>
>  
>  #include "test.h"
>  
> @@ -31,6 +32,9 @@
>  
>  #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
>  
> +/* Use jhash or crc hash */
> +#define USE_JHASH 1
> +
>  struct {
>  	uint32_t *keys;
>  	uint32_t *found;
> @@ -108,10 +112,14 @@ test_hash_multiwriter(void)
>  	struct rte_hash_parameters hash_params = {
>  		.entries = nb_entries,
>  		.key_len = sizeof(uint32_t),
> -		.hash_func = rte_hash_crc,
>  		.hash_func_init_val = 0,
>  		.socket_id = rte_socket_id(),
>  	};
> +	if (USE_JHASH)
> +		hash_params.hash_func = rte_jhash;
> +	else
> +		hash_params.hash_func = rte_hash_crc;
> +
>  	if (use_htm)
>  		hash_params.extra_flag =
>  			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
> -- 
As I commented on v1, rather than having a macro at the top hard-coded to
jhash, why not just do a straight replacement of crc to jhash in the
structure definition. Since changing the hash function will involve editing
the source code anyway, I see little point in using the macro at the top -
especially since there is no indication to the user what effect removing it
or changing it to zero has.

/Bruce

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table Yipeng Wang
@ 2018-10-25  9:34   ` Bruce Richardson
  2018-10-26  0:32   ` Honnappa Nagarahalli
  1 sibling, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2018-10-25  9:34 UTC (permalink / raw)
  To: Yipeng Wang; +Cc: stephen, dev, honnappa.nagarahalli, sameh.gobriel

On Wed, Oct 24, 2018 at 11:09:29AM -0700, Yipeng Wang wrote:
> This commit improves the readwrite test to consider
> extendable table feature and add more functional tests
> to cover more corner cases.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test Yipeng Wang
@ 2018-10-25  9:34   ` Bruce Richardson
  0 siblings, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2018-10-25  9:34 UTC (permalink / raw)
  To: Yipeng Wang; +Cc: stephen, dev, honnappa.nagarahalli, sameh.gobriel

On Wed, Oct 24, 2018 at 11:09:30AM -0700, Yipeng Wang wrote:
> The hash scaling unit test is not really needed
> any more since the multi-writer is supported now
> inside the library and it is tested by multi-writer
> unit test.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/Makefile            |   1 -
>  test/test/autotest_data.py    |   6 --
>  test/test/meson.build         |   2 -
>  test/test/test_hash_scaling.c | 191 ------------------------------------------
>  4 files changed, 200 deletions(-)
>  delete mode 100644 test/test/test_hash_scaling.c
> 
I always like having less code that needs maintenance! :-)

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places
  2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
                   ` (3 preceding siblings ...)
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test Yipeng Wang
@ 2018-10-25 19:11 ` Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause Yipeng Wang
                     ` (6 more replies)
  4 siblings, 7 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: cover more test case in unit test.
commit 4: improve readwrite test to consider the extendable table.
commit 5: remove scaling unit test.
commit 6: add readwrite test to meson and autotest file.

v2->v3:
* Split commit 4 to commit 3 and 4 (Honnappa)
* Remove hard coded macro (Bruce)
* Add commit 6 fix to add readwrite test to autotest files.
* Remove unnecessary header in commit 1 (Mattias)

V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (6):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: test more corner cases in unit test
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test
  test/hash: fix to add read-write test to autotest

 lib/librte_hash/rte_cuckoo_hash.c |   5 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |  12 +--
 test/test/meson.build             |   3 +-
 test/test/test_hash_multiwriter.c |   3 +-
 test/test/test_hash_readwrite.c   |  73 ++++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 70 insertions(+), 218 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..5ddcccd 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -27,7 +27,6 @@
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
-#include <rte_pause.h>
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -574,14 +573,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 2/6] test/hash: change multiwriter test to use jhash
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test Yipeng Wang
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..d447f6d 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -108,7 +109,7 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
+		.hash_func = rte_jhash,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-26  5:03     ` Honnappa Nagarahalli
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 4/6] test/hash: add readwrite test for ext table Yipeng Wang
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This commit improves the readwrite unit test to cover
more corner cases and reduces the testing time by
reducing the total key count.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..a45c669 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -18,8 +18,8 @@
 
 #define RTE_RWTEST_FAIL 0
 
-#define TOTAL_ENTRY (16*1024*1024)
-#define TOTAL_INSERT (15*1024*1024)
+#define TOTAL_ENTRY (5*1024*1024)
+#define TOTAL_INSERT (4.5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -59,8 +59,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 4/6] test/hash: add readwrite test for ext table
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
                     ` (2 preceding siblings ...)
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 5/6] test/hash: remove hash scaling unit test Yipeng Wang
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This commit improves the readwrite test to consider
extendable table feature.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 42 +++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a45c669..8cbe85bd 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -20,6 +20,7 @@
 
 #define TOTAL_ENTRY (5*1024*1024)
 #define TOTAL_INSERT (4.5*1024*1024)
+#define TOTAL_INSERT_EXT (5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -119,7 +120,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -148,6 +149,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -186,7 +194,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -197,6 +205,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -204,11 +213,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -364,7 +378,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(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -598,7 +612,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -621,7 +635,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(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;
@@ -640,8 +660,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(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)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 5/6] test/hash: remove hash scaling unit test
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
                     ` (3 preceding siblings ...)
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 4/6] test/hash: add readwrite test for ext table Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 1b5c070..6b77e15 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -113,7 +113,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 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_scaling.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
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 4eae588..9265617 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index faef5a4..7ebe9ab 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -170,7 +169,6 @@ test_names = [
 	'external_mem_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v3 6/6] test/hash: fix to add read-write test to autotest
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
                     ` (4 preceding siblings ...)
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 5/6] test/hash: remove hash scaling unit test Yipeng Wang
@ 2018-10-25 19:11   ` Yipeng Wang
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-25 19:11 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

Add read-write concurrency test to meson and autotest
file.

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/autotest_data.py | 6 ++++++
 test/test/meson.build      | 1 +
 2 files changed, 7 insertions(+)

diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 9265617..0df686a 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -574,6 +574,12 @@
         "Report":  None,
     },
     {
+        "Name":    "Hash read-write concurrency autotest",
+        "Command": "hash_readwrite_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
         "Name":    "Hash read-write lock-free concurrency autotest",
         "Command": "hash_readwrite_lf_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index 7ebe9ab..1c1fcd7 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -173,6 +173,7 @@ test_names = [
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
 	'hash_perf_autotest',
+	'hash_readwrite_autotest',
 	'hash_readwrite_lf_autotest',
 	'interrupt_autotest',
 	'kni_autotest',
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
  2018-10-25  6:31   ` Mattias Rönnblom
@ 2018-10-26  0:24   ` Honnappa Nagarahalli
  2018-10-26  2:04     ` Wang, Yipeng1
  1 sibling, 1 reply; 31+ messages in thread
From: Honnappa Nagarahalli @ 2018-10-26  0:24 UTC (permalink / raw)
  To: Yipeng Wang, bruce.richardson; +Cc: stephen, dev, sameh.gobriel, nd

> 
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared data structure, the rte_pause
> is not needed.
> 
> Fixes: b26473ff8f4a ("hash: add reset function")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index 0648a22..4a2647e 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> 
>  	/* clear the free ring */
>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> -		rte_pause();
> +		continue;
Minor comment: 'continue' can be removed.

> 
>  	/* clear free extendable bucket ring and memory */
>  	if (h->ext_table_support) {
>  		memset(h->buckets_ext, 0, h->num_buckets *
>  						sizeof(struct
> rte_hash_bucket));
>  		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
> -			rte_pause();
> +			continue;
>  	}
> 
>  	/* Repopulate the free slots ring. Entry zero is reserved for key
> misses */
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table
  2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table Yipeng Wang
  2018-10-25  9:34   ` Bruce Richardson
@ 2018-10-26  0:32   ` Honnappa Nagarahalli
  1 sibling, 0 replies; 31+ messages in thread
From: Honnappa Nagarahalli @ 2018-10-26  0:32 UTC (permalink / raw)
  To: Yipeng Wang, bruce.richardson; +Cc: stephen, dev, sameh.gobriel, nd

> Subject: [PATCH v2 3/4] test/hash: add readwrite test for ext table
> 
> This commit improves the readwrite test to consider extendable table
> feature and add more functional tests to cover more corner cases.
> 
I think the part covering the more corner cases should go into a separate commit.

> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_readwrite.c | 70
> ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/test/test/test_hash_readwrite.c
> b/test/test/test_hash_readwrite.c index a8fadd0..13c28e0 100644
> --- a/test/test/test_hash_readwrite.c
> +++ b/test/test/test_hash_readwrite.c
> @@ -21,6 +21,8 @@
>  #define TOTAL_ENTRY (16*1024*1024)
>  #define TOTAL_INSERT (15*1024*1024)
> 
> +#define TOTAL_INSERT_EXT (16*1024*1024)
> +
>  #define NUM_TEST 3
>  unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
> 
> @@ -59,8 +61,10 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  	uint64_t i, offset;
>  	uint32_t lcore_id = rte_lcore_id();
>  	uint64_t begin, cycles;
> -	int ret;
> +	int *ret;
> 
> +	ret = rte_malloc(NULL, sizeof(int) *
> +				tbl_rw_test_param.num_insert, 0);
This memory needs to be freed.

>  	for (i = 0; i < rte_lcore_count(); i++) {
>  		if (slave_core_ids[i] == lcore_id)
>  			break;
> @@ -79,13 +83,30 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  				tbl_rw_test_param.keys + i) > 0)
>  			break;
> 
> -		ret = rte_hash_add_key(tbl_rw_test_param.h,
> +		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
>  				     tbl_rw_test_param.keys + i);
> -		if (ret < 0)
> +		if (ret[i - offset] < 0)
> +			break;
> +
> +		/* lookup a random key */
> +		uint32_t rand = rte_rand() % (i + 1 - offset);
> +
> +		if (rte_hash_lookup(tbl_rw_test_param.h,
> +				tbl_rw_test_param.keys + rand) != ret[rand])
> +			break;
> +
> +
> +		if (rte_hash_del_key(tbl_rw_test_param.h,
> +				tbl_rw_test_param.keys + rand) != ret[rand])
> +			break;
> +
> +		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
> +					tbl_rw_test_param.keys + rand);
> +		if (ret[rand] < 0)
>  			break;
> 
>  		if (rte_hash_lookup(tbl_rw_test_param.h,
> -				tbl_rw_test_param.keys + i) != ret)
> +			tbl_rw_test_param.keys + rand) != ret[rand])
>  			break;
>  	}
> 
> @@ -100,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)  }
> 
>  static int
> -init_params(int use_htm, int use_jhash)
> +init_params(int use_ext, int use_htm, int use_jhash)
>  {
>  	unsigned int i;
> 
> @@ -129,6 +150,13 @@ init_params(int use_htm, int use_jhash)
>  			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>  			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> 
> +	if (use_ext)
> +		hash_params.extra_flag |=
> +			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
> +	else
> +		hash_params.extra_flag &=
> +		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
> +
>  	hash_params.name = "tests";
> 
>  	handle = rte_hash_create(&hash_params); @@ -167,7 +195,7 @@
> init_params(int use_htm, int use_jhash)  }
> 
>  static int
> -test_hash_readwrite_functional(int use_htm)
> +test_hash_readwrite_functional(int use_ext, int use_htm)
>  {
>  	unsigned int i;
>  	const void *next_key;
> @@ -178,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
>  	uint32_t lost_keys = 0;
>  	int use_jhash = 1;
>  	int slave_cnt = rte_lcore_count() - 1;
> +	uint32_t tot_insert = 0;
> 
>  	rte_atomic64_init(&gcycles);
>  	rte_atomic64_clear(&gcycles);
> @@ -185,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
>  	rte_atomic64_init(&ginsertions);
>  	rte_atomic64_clear(&ginsertions);
> 
> -	if (init_params(use_htm, use_jhash) != 0)
> +	if (init_params(use_ext, use_htm, use_jhash) != 0)
>  		goto err;
> 
> +	if (use_ext)
> +		tot_insert = TOTAL_INSERT_EXT;
> +	else
> +		tot_insert = TOTAL_INSERT;
> +
>  	tbl_rw_test_param.num_insert =
> -		TOTAL_INSERT / slave_cnt;
> +		tot_insert / slave_cnt;
> 
>  	tbl_rw_test_param.rounded_tot_insert =
>  		tbl_rw_test_param.num_insert
> @@ -345,7 +379,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(use_htm, use_jhash) != 0)
> +	if (init_params(0, use_htm, use_jhash) != 0)
>  		goto err;
> 
>  	/*
> @@ -579,7 +613,7 @@ test_hash_readwrite_main(void)
>  	 * than writer threads. This is to timing either reader threads or
>  	 * writer threads for performance numbers.
>  	 */
> -	int use_htm, reader_faster;
> +	int use_htm, use_ext,  reader_faster;
>  	unsigned int i = 0, core_id = 0;
> 
>  	if (rte_lcore_count() <= 2) {
> @@ -602,7 +636,13 @@ test_hash_readwrite_main(void)
>  		printf("Test read-write with Hardware transactional
> memory\n");
> 
>  		use_htm = 1;
> -		if (test_hash_readwrite_functional(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;
> @@ -621,8 +661,14 @@ test_hash_readwrite_main(void)
> 
>  	printf("Test read-write without Hardware transactional memory\n");
>  	use_htm = 0;
> -	if (test_hash_readwrite_functional(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)
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-26  0:24   ` Honnappa Nagarahalli
@ 2018-10-26  2:04     ` Wang, Yipeng1
  2018-10-26 11:09       ` Bruce Richardson
  0 siblings, 1 reply; 31+ messages in thread
From: Wang, Yipeng1 @ 2018-10-26  2:04 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Richardson, Bruce; +Cc: stephen, dev, Gobriel, Sameh, nd

>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>> ---
>>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
>> b/lib/librte_hash/rte_cuckoo_hash.c
>> index 0648a22..4a2647e 100644
>> --- a/lib/librte_hash/rte_cuckoo_hash.c
>> +++ b/lib/librte_hash/rte_cuckoo_hash.c
>> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
>>
>>  	/* clear the free ring */
>>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
>> -		rte_pause();
>> +		continue;
>Minor comment: 'continue' can be removed.
>
[Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
Anyone know a common way to do it in DPDK?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test Yipeng Wang
@ 2018-10-26  5:03     ` Honnappa Nagarahalli
  2018-10-26 11:02       ` Thomas Monjalon
  0 siblings, 1 reply; 31+ messages in thread
From: Honnappa Nagarahalli @ 2018-10-26  5:03 UTC (permalink / raw)
  To: Yipeng Wang, bruce.richardson; +Cc: stephen, dev, sameh.gobriel

>
> This commit improves the readwrite unit test to cover more corner cases and
> reduces the testing time by reducing the total key count.
>
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  test/test/test_hash_readwrite.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
> index a8fadd0..a45c669 100644
> --- a/test/test/test_hash_readwrite.c
> +++ b/test/test/test_hash_readwrite.c
> @@ -18,8 +18,8 @@
>
>  #define RTE_RWTEST_FAIL 0
>
> -#define TOTAL_ENTRY (16*1024*1024)
> -#define TOTAL_INSERT (15*1024*1024)
> +#define TOTAL_ENTRY (5*1024*1024)
> +#define TOTAL_INSERT (4.5*1024*1024)
>
>  #define NUM_TEST 3
>  unsigned int core_cnt[NUM_TEST] = {2, 4, 8}; @@ -59,8 +59,10 @@
> test_hash_readwrite_worker(__attribute__((unused)) void *arg)
>  uint64_t i, offset;
>  uint32_t lcore_id = rte_lcore_id();
>  uint64_t begin, cycles;
> -int ret;
> +int *ret;
>
> +ret = rte_malloc(NULL, sizeof(int) *
> +tbl_rw_test_param.num_insert, 0);
This memory needs to be freed at the end of this function

>  for (i = 0; i < rte_lcore_count(); i++) {
>  if (slave_core_ids[i] == lcore_id)
>  break;
> @@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  tbl_rw_test_param.keys + i) > 0)
>  break;
>
> -ret = rte_hash_add_key(tbl_rw_test_param.h,
> +ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
>       tbl_rw_test_param.keys + i);
> -if (ret < 0)
> +if (ret[i - offset] < 0)
> +break;
> +
> +/* lookup a random key */
> +uint32_t rand = rte_rand() % (i + 1 - offset);
> +
> +if (rte_hash_lookup(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand) != ret[rand])
> +break;
> +
> +
> +if (rte_hash_del_key(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand) != ret[rand])
> +break;
> +
> +ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand);
> +if (ret[rand] < 0)
>  break;
>
>  if (rte_hash_lookup(tbl_rw_test_param.h,
> -tbl_rw_test_param.keys + i) != ret)
> +tbl_rw_test_param.keys + rand) != ret[rand])
>  break;
>  }
>
> --
> 2.7.4

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places
  2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
                     ` (5 preceding siblings ...)
  2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
@ 2018-10-26  9:53   ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause Yipeng Wang
                       ` (6 more replies)
  6 siblings, 7 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: cover more test case in unit test.
commit 4: improve readwrite test to consider the extendable table.
commit 5: remove scaling unit test.
commit 6: add readwrite test to meson and autotest file.

v3->v4:
* Fix memory leak in commit 3 (Honnappa)

v2->v3:
* Split commit 4 to commit 3 and 4 (Honnappa)
* Remove hard coded macro (Bruce)
* Add commit 6 fix to add readwrite test to autotest files.
* Remove unnecessary header in commit 1 (Mattias)

V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (6):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: test more corner cases in unit test
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test
  test/hash: fix to add read-write test to autotest

 lib/librte_hash/rte_cuckoo_hash.c |   5 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |  12 +--
 test/test/meson.build             |   3 +-
 test/test/test_hash_multiwriter.c |   3 +-
 test/test/test_hash_readwrite.c   |  74 ++++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 71 insertions(+), 218 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
                       ` (5 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..5ddcccd 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -27,7 +27,6 @@
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
-#include <rte_pause.h>
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -574,14 +573,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 2/6] test/hash: change multiwriter test to use jhash
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 3/6] test/hash: test more corner cases in unit test Yipeng Wang
                       ` (4 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..d447f6d 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -108,7 +109,7 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
+		.hash_func = rte_jhash,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 3/6] test/hash: test more corner cases in unit test
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 4/6] test/hash: add readwrite test for ext table Yipeng Wang
                       ` (3 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This commit improves the readwrite unit test to cover
more corner cases and reduces the testing time by
reducing the total key count.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..26e802c 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -18,8 +18,8 @@
 
 #define RTE_RWTEST_FAIL 0
 
-#define TOTAL_ENTRY (16*1024*1024)
-#define TOTAL_INSERT (15*1024*1024)
+#define TOTAL_ENTRY (5*1024*1024)
+#define TOTAL_INSERT (4.5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -59,8 +59,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
@@ -96,6 +115,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	for (; i < offset + tbl_rw_test_param.num_insert; i++)
 		tbl_rw_test_param.keys[i] = RTE_RWTEST_FAIL;
 
+	rte_free(ret);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 4/6] test/hash: add readwrite test for ext table
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
                       ` (2 preceding siblings ...)
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 3/6] test/hash: test more corner cases in unit test Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 5/6] test/hash: remove hash scaling unit test Yipeng Wang
                       ` (2 subsequent siblings)
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

This commit improves the readwrite test to consider
extendable table feature.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 42 +++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 26e802c..01f986c 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -20,6 +20,7 @@
 
 #define TOTAL_ENTRY (5*1024*1024)
 #define TOTAL_INSERT (4.5*1024*1024)
+#define TOTAL_INSERT_EXT (5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -120,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -149,6 +150,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -187,7 +195,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -198,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -205,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -365,7 +379,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(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -599,7 +613,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -622,7 +636,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(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;
@@ -641,8 +661,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(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)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 5/6] test/hash: remove hash scaling unit test
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
                       ` (3 preceding siblings ...)
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 4/6] test/hash: add readwrite test for ext table Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
  2018-10-26 20:15     ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Thomas Monjalon
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 1b5c070..6b77e15 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -113,7 +113,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 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_scaling.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
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 4eae588..9265617 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index faef5a4..7ebe9ab 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -170,7 +169,6 @@ test_names = [
 	'external_mem_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [dpdk-dev] [PATCH v4 6/6] test/hash: fix to add read-write test to autotest
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
                       ` (4 preceding siblings ...)
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 5/6] test/hash: remove hash scaling unit test Yipeng Wang
@ 2018-10-26  9:53     ` Yipeng Wang
  2018-10-26 20:15     ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Thomas Monjalon
  6 siblings, 0 replies; 31+ messages in thread
From: Yipeng Wang @ 2018-10-26  9:53 UTC (permalink / raw)
  To: bruce.richardson
  Cc: stephen, dev, yipeng1.wang, honnappa.nagarahalli, sameh.gobriel

Add read-write concurrency test to meson and autotest
file.

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/autotest_data.py | 6 ++++++
 test/test/meson.build      | 1 +
 2 files changed, 7 insertions(+)

diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 9265617..0df686a 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -574,6 +574,12 @@
         "Report":  None,
     },
     {
+        "Name":    "Hash read-write concurrency autotest",
+        "Command": "hash_readwrite_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
         "Name":    "Hash read-write lock-free concurrency autotest",
         "Command": "hash_readwrite_lf_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index 7ebe9ab..1c1fcd7 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -173,6 +173,7 @@ test_names = [
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
 	'hash_perf_autotest',
+	'hash_readwrite_autotest',
 	'hash_readwrite_lf_autotest',
 	'interrupt_autotest',
 	'kni_autotest',
-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test
  2018-10-26  5:03     ` Honnappa Nagarahalli
@ 2018-10-26 11:02       ` Thomas Monjalon
  0 siblings, 0 replies; 31+ messages in thread
From: Thomas Monjalon @ 2018-10-26 11:02 UTC (permalink / raw)
  To: Yipeng Wang
  Cc: dev, Honnappa Nagarahalli, bruce.richardson, stephen, sameh.gobriel

> > --- a/test/test/test_hash_readwrite.c
> > +++ b/test/test/test_hash_readwrite.c
> > test_hash_readwrite_worker(__attribute__((unused)) void *arg)
> >  uint64_t i, offset;
> >  uint32_t lcore_id = rte_lcore_id();
> >  uint64_t begin, cycles;
> > -int ret;
> > +int *ret;
> >
> > +ret = rte_malloc(NULL, sizeof(int) *
> > +tbl_rw_test_param.num_insert, 0);
> This memory needs to be freed at the end of this function

I am waiting a v4 for this change, please.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause
  2018-10-26  2:04     ` Wang, Yipeng1
@ 2018-10-26 11:09       ` Bruce Richardson
  0 siblings, 0 replies; 31+ messages in thread
From: Bruce Richardson @ 2018-10-26 11:09 UTC (permalink / raw)
  To: Wang, Yipeng1; +Cc: Honnappa Nagarahalli, stephen, dev, Gobriel, Sameh, nd

On Fri, Oct 26, 2018 at 03:04:12AM +0100, Wang, Yipeng1 wrote:
> >-----Original Message-----
> >From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
> >> ---
> >>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> >> b/lib/librte_hash/rte_cuckoo_hash.c
> >> index 0648a22..4a2647e 100644
> >> --- a/lib/librte_hash/rte_cuckoo_hash.c
> >> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> >> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> >>
> >>  	/* clear the free ring */
> >>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> >> -		rte_pause();
> >> +		continue;
> >Minor comment: 'continue' can be removed.
> >
> [Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
> Anyone know a common way to do it in DPDK?
> 
Semi-colon on a new line, possibly with comment should work ok. However,
the continue is harmless, so I'm ok with it as-is.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places
  2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
                       ` (5 preceding siblings ...)
  2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
@ 2018-10-26 20:15     ` Thomas Monjalon
  6 siblings, 0 replies; 31+ messages in thread
From: Thomas Monjalon @ 2018-10-26 20:15 UTC (permalink / raw)
  To: Yipeng Wang
  Cc: dev, bruce.richardson, stephen, honnappa.nagarahalli, sameh.gobriel

> Yipeng Wang (6):
>   hash: fix unnecessary pause
>   test/hash: change multiwriter test to use jhash
>   test/hash: test more corner cases in unit test
>   test/hash: add readwrite test for ext table
>   test/hash: remove hash scaling unit test
>   test/hash: fix to add read-write test to autotest

Applied, thanks

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2018-10-26 20:15 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24 18:09 [dpdk-dev] [PATCH v2 0/4] hash: improve multiple places Yipeng Wang
2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause Yipeng Wang
2018-10-25  6:31   ` Mattias Rönnblom
2018-10-25  9:30     ` Bruce Richardson
2018-10-26  0:24   ` Honnappa Nagarahalli
2018-10-26  2:04     ` Wang, Yipeng1
2018-10-26 11:09       ` Bruce Richardson
2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash Yipeng Wang
2018-10-25  9:32   ` Bruce Richardson
2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table Yipeng Wang
2018-10-25  9:34   ` Bruce Richardson
2018-10-26  0:32   ` Honnappa Nagarahalli
2018-10-24 18:09 ` [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test Yipeng Wang
2018-10-25  9:34   ` Bruce Richardson
2018-10-25 19:11 ` [dpdk-dev] [PATCH v3 0/6] hash: improve multiple places Yipeng Wang
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause Yipeng Wang
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test Yipeng Wang
2018-10-26  5:03     ` Honnappa Nagarahalli
2018-10-26 11:02       ` Thomas Monjalon
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 4/6] test/hash: add readwrite test for ext table Yipeng Wang
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 5/6] test/hash: remove hash scaling unit test Yipeng Wang
2018-10-25 19:11   ` [dpdk-dev] [PATCH v3 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
2018-10-26  9:53   ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 2/6] test/hash: change multiwriter test to use jhash Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 3/6] test/hash: test more corner cases in unit test Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 4/6] test/hash: add readwrite test for ext table Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 5/6] test/hash: remove hash scaling unit test Yipeng Wang
2018-10-26  9:53     ` [dpdk-dev] [PATCH v4 6/6] test/hash: fix to add read-write test to autotest Yipeng Wang
2018-10-26 20:15     ` [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places 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).