[dpdk-dev] [PATCH v2 2/4] hash: deprecate lock ellision and read/write concurreny flags

[Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>]

Hash library should provide read/write concurrency by default
as most of the use cases require read/write concurrency. Hence
the flag RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY is deprecated.
The library will decide if locking is required to provide
the concurrency based on other configuration flags.

If a lock is used to provide the read/write concurrency, best
possible locking mechanism available on the platform should
be used. Hence, the flag RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
is deprecated. The library will use transactional memory
if the platform supports it.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
---
 lib/librte_hash/rte_cuckoo_hash.c    | 46 +++++++++++-----------------
 lib/librte_hash/rte_hash.h           | 20 ++++++++++--
 lib/librte_hash/rte_hash_version.map |  7 +++++
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index ec3b519ba..7f97cddd4 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -121,7 +121,7 @@ get_alt_bucket_index(const struct rte_hash *h,
 }
 
 struct rte_hash *
-rte_hash_create(const struct rte_hash_parameters *params)
+rte_hash_create_v20(const struct rte_hash_parameters *params)
 {
 	struct rte_hash *h = NULL;
 	struct rte_tailq_entry *te = NULL;
@@ -446,6 +446,7 @@ rte_hash_create(const struct rte_hash_parameters *params)
 	rte_free(tbl_chng_cnt);
 	return NULL;
 }
+VERSION_SYMBOL(rte_hash_create, _v20, 2.0);
 
 struct rte_hash *
 rte_hash_create_v1811(const struct rte_hash_parameters *params)
@@ -463,10 +464,10 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 	char ext_ring_name[RTE_RING_NAMESIZE];
 	unsigned num_key_slots;
 	unsigned i;
-	unsigned int hw_trans_mem_support = 0, use_local_cache = 0;
+	unsigned int use_local_cache = 0;
 	unsigned int ext_table_support = 0;
-	unsigned int readwrite_concur_support = 0;
-	unsigned int writer_takes_lock = 0;
+	unsigned int readwrite_concur_support = 1;
+	unsigned int writer_takes_lock = 1;
 	unsigned int no_free_on_del = 0;
 	uint32_t *tbl_chng_cnt = NULL;
 	unsigned int readwrite_concur_lf_support = 0;
@@ -490,14 +491,6 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 	}
 
 	/* Validate correct usage of extra options */
-	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) &&
-	    (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)) {
-		rte_errno = EINVAL;
-		RTE_LOG(ERR, HASH, "rte_hash_create: choose rw concurrency or "
-			"rw concurrency lock free\n");
-		return NULL;
-	}
-
 	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF) &&
 	    (params->extra_flag & RTE_HASH_EXTRA_FLAGS_EXT_TABLE)) {
 		rte_errno = EINVAL;
@@ -506,18 +499,8 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 	}
 
 	/* Check extra flags field to check extra options. */
-	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT)
-		hw_trans_mem_support = 1;
-
-	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD) {
+	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD)
 		use_local_cache = 1;
-		writer_takes_lock = 1;
-	}
-
-	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) {
-		readwrite_concur_support = 1;
-		writer_takes_lock = 1;
-	}
 
 	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_EXT_TABLE)
 		ext_table_support = 1;
@@ -526,11 +509,17 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 		no_free_on_del = 1;
 
 	if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF) {
+		/* Do not use lock for RW concurrency */
+		readwrite_concur_support = 0;
 		readwrite_concur_lf_support = 1;
 		/* Enable not freeing internal memory/index on delete */
 		no_free_on_del = 1;
 	}
 
+	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF) &&
+	    !(params->extra_flag & RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD))
+		writer_takes_lock = 0;
+
 	/* Store all keys and leave the first entry as a dummy entry
 	 * for lookup_bulk.
 	 */
@@ -725,7 +714,7 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 	h->free_slots = r;
 	h->tbl_chng_cnt = tbl_chng_cnt;
 	*h->tbl_chng_cnt = 0;
-	h->hw_trans_mem_support = hw_trans_mem_support;
+	h->hw_trans_mem_support = rte_tm_supported();
 	h->use_local_cache = use_local_cache;
 	h->readwrite_concur_support = readwrite_concur_support;
 	h->ext_table_support = ext_table_support;
@@ -740,10 +729,6 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 #endif
 		h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;
 
-	/* Writer threads need to take the lock when:
-	 * 1) RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY is enabled OR
-	 * 2) RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD is enabled
-	 */
 	if (h->writer_takes_lock) {
 		h->readwrite_lock = rte_malloc(NULL, sizeof(rte_rwlock_t),
 						RTE_CACHE_LINE_SIZE);
@@ -775,6 +760,11 @@ rte_hash_create_v1811(const struct rte_hash_parameters *params)
 	rte_free(tbl_chng_cnt);
 	return NULL;
 }
+BIND_DEFAULT_SYMBOL(rte_hash_create, _v1811, 18.11);
+MAP_STATIC_SYMBOL(
+	struct rte_hash *rte_hash_create(
+		const struct rte_hash_parameters *params),
+		rte_hash_create_v1811);
 
 void
 rte_hash_free(struct rte_hash *h)
diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index f049b9dd0..93c7019ec 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -30,13 +30,27 @@ extern "C" {
 #define RTE_HASH_LOOKUP_BULK_MAX		64
 #define RTE_HASH_LOOKUP_MULTI_MAX		RTE_HASH_LOOKUP_BULK_MAX
 
-/** Enable Hardware transactional memory support. */
+/**
+ * @deprecated
+ * This define will be removed in the next release.
+ * If the target platform supports hardware transactional memory
+ * it will be used without user consent as it provides the best possible
+ * performance.
+ *
+ * Enable Hardware transactional memory support.
+ */
 #define RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT	0x01
 
 /** Default behavior of insertion, single writer/multi writer */
 #define RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD 0x02
 
-/** Flag to support reader writer concurrency */
+/**
+ * @deprecated
+ * This define will be removed in the next release.
+ * This library should be thread-safe by default.
+ *
+ * Flag to support reader writer concurrency
+ */
 #define RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY 0x04
 
 /** Flag to indicate the extendabe bucket table feature should be used */
@@ -106,6 +120,8 @@ struct rte_hash;
 struct rte_hash *
 rte_hash_create(const struct rte_hash_parameters *params);
 struct rte_hash *
+rte_hash_create_v20(const struct rte_hash_parameters *params);
+struct rte_hash *
 rte_hash_create_v1811(const struct rte_hash_parameters *params);
 
 /**
diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map
index 734ae28b0..c72469099 100644
--- a/lib/librte_hash/rte_hash_version.map
+++ b/lib/librte_hash/rte_hash_version.map
@@ -54,6 +54,13 @@ DPDK_18.08 {
 
 } DPDK_16.07;
 
+DPDK_18.11 {
+	global:
+
+	rte_hash_create;
+
+} DPDK_18.08;
+
 EXPERIMENTAL {
 	global:
 
-- 
2.17.1