[dpdk-dev,v1,3/3] hash: add new API function to query the key count

Message ID 1528455078-328182-4-git-send-email-yipeng1.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Add read-write concurrency to rte_hash library |

Checks

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

Commit Message

Wang, Yipeng1 June 8, 2018, 10:51 a.m. UTC
  Add a new function, rte_hash_count, to return the number of keys that
are currently stored in the hash table. Corresponding test functions are
added into hash_test and hash_multiwriter test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c    | 39 +++++++++++++++++++++++++++++++-----
 lib/librte_hash/rte_hash.h           | 11 ++++++++++
 lib/librte_hash/rte_hash_version.map |  8 ++++++++
 test/test/test_hash.c                | 12 +++++++++++
 test/test/test_hash_multiwriter.c    |  9 +++++++++
 5 files changed, 74 insertions(+), 5 deletions(-)
  

Comments

De Lara Guarch, Pablo June 26, 2018, 4:11 p.m. UTC | #1
> -----Original Message-----
> From: Wang, Yipeng1
> Sent: Friday, June 8, 2018 11:51 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Mcnamara,
> John <john.mcnamara@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; honnappa.nagarahalli@arm.com;
> vguvva@caviumnetworks.com; brijesh.s.singh@gmail.com
> Subject: [PATCH v1 3/3] hash: add new API function to query the key count
> 
> Add a new function, rte_hash_count, to return the number of keys that are
> currently stored in the hash table. Corresponding test functions are added into
> hash_test and hash_multiwriter test.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c    | 39
> +++++++++++++++++++++++++++++++-----
>  lib/librte_hash/rte_hash.h           | 11 ++++++++++
>  lib/librte_hash/rte_hash_version.map |  8 ++++++++
>  test/test/test_hash.c                | 12 +++++++++++
>  test/test/test_hash_multiwriter.c    |  9 +++++++++
>  5 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index a5bb4d4..3dff871 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -133,13 +133,13 @@ rte_hash_create(const struct rte_hash_parameters
> *params)
>  		 * except for the first cache
>  		 */
>  		num_key_slots = params->entries + (RTE_MAX_LCORE - 1) *
> -					LCORE_CACHE_SIZE + 1;
> +					(LCORE_CACHE_SIZE - 1) + 1;

This and the other changes made outside the new rte_hash_count API can be done in a different patch.
If this was an issue on the calculation of key slots, it should be marked as a fix and then
a patch with the new API can follow, with the tests.

...

> 
> +int32_t
> +rte_hash_count(struct rte_hash *h)
> +{
> +	uint32_t tot_ring_cnt, cached_cnt = 0;
> +	uint32_t i, ret;
> +
> +	if (h == NULL || h->free_slots == NULL)

I don't think the check on free_slots is necessary,
since rte_hash_create is already checking that.

...

> --- a/lib/librte_hash/rte_hash.h
> +++ b/lib/librte_hash/rte_hash.h
> @@ -127,6 +127,17 @@ void
>  rte_hash_reset(struct rte_hash *h);
> 
>  /**
> + * Return the number of keys in the hash table
> + * @param h
> + *  Hash table to query from
> + * @return
> + *   - -EINVAL if parameters are invalid
> + *   - A value indicating how many keys inserted in the table.

"how many keys were inserted"
  

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index a5bb4d4..3dff871 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -133,13 +133,13 @@  rte_hash_create(const struct rte_hash_parameters *params)
 		 * except for the first cache
 		 */
 		num_key_slots = params->entries + (RTE_MAX_LCORE - 1) *
-					LCORE_CACHE_SIZE + 1;
+					(LCORE_CACHE_SIZE - 1) + 1;
 	else
 		num_key_slots = params->entries + 1;
 
 	snprintf(ring_name, sizeof(ring_name), "HT_%s", params->name);
 	/* Create ring (Dummy slot index is not enqueued) */
-	r = rte_ring_create(ring_name, rte_align32pow2(num_key_slots - 1),
+	r = rte_ring_create(ring_name, rte_align32pow2(num_key_slots),
 			params->socket_id, 0);
 	if (r == NULL) {
 		RTE_LOG(ERR, HASH, "memory allocation failed\n");
@@ -290,7 +290,7 @@  rte_hash_create(const struct rte_hash_parameters *params)
 	}
 
 	/* Populate free slots ring. Entry zero is reserved for key misses. */
-	for (i = 1; i < params->entries + 1; i++)
+	for (i = 1; i < num_key_slots; i++)
 		rte_ring_sp_enqueue(r, (void *)((uintptr_t) i));
 
 	te->data = (void *) h;
@@ -371,7 +371,7 @@  void
 rte_hash_reset(struct rte_hash *h)
 {
 	void *ptr;
-	unsigned i;
+	uint32_t tot_ring_cnt, i;
 
 	if (h == NULL)
 		return;
@@ -384,7 +384,13 @@  rte_hash_reset(struct rte_hash *h)
 		rte_pause();
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-	for (i = 1; i < h->entries + 1; i++)
+	if (h->multi_writer_support)
+		tot_ring_cnt = h->entries + (RTE_MAX_LCORE - 1) *
+					(LCORE_CACHE_SIZE - 1);
+	else
+		tot_ring_cnt = h->entries;
+
+	for (i = 1; i < tot_ring_cnt + 1; i++)
 		rte_ring_sp_enqueue(h->free_slots, (void *)((uintptr_t) i));
 
 	if (h->multi_writer_support) {
@@ -394,6 +400,29 @@  rte_hash_reset(struct rte_hash *h)
 	}
 }
 
+int32_t
+rte_hash_count(struct rte_hash *h)
+{
+	uint32_t tot_ring_cnt, cached_cnt = 0;
+	uint32_t i, ret;
+
+	if (h == NULL || h->free_slots == NULL)
+		return -EINVAL;
+
+	if (h->multi_writer_support) {
+		tot_ring_cnt = h->entries + (RTE_MAX_LCORE - 1) *
+					(LCORE_CACHE_SIZE - 1);
+		for (i = 0; i < RTE_MAX_LCORE; i++)
+			cached_cnt += h->local_free_slots[i].len;
+
+		ret = tot_ring_cnt - rte_ring_count(h->free_slots) -
+								cached_cnt;
+	} else {
+		tot_ring_cnt = h->entries;
+		ret = tot_ring_cnt - rte_ring_count(h->free_slots);
+	}
+	return ret;
+}
 
 static inline void
 __hash_rw_writer_lock(const struct rte_hash *h)
diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index ecb49e4..b8f1f31 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -127,6 +127,17 @@  void
 rte_hash_reset(struct rte_hash *h);
 
 /**
+ * Return the number of keys in the hash table
+ * @param h
+ *  Hash table to query from
+ * @return
+ *   - -EINVAL if parameters are invalid
+ *   - A value indicating how many keys inserted in the table.
+ */
+int32_t
+rte_hash_count(struct rte_hash *h);
+
+/**
  * Add a key-value pair to an existing hash table.
  * This operation is not multi-thread safe
  * and should only be called from one thread.
diff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map
index 52a2576..e216ac8 100644
--- a/lib/librte_hash/rte_hash_version.map
+++ b/lib/librte_hash/rte_hash_version.map
@@ -45,3 +45,11 @@  DPDK_16.07 {
 	rte_hash_get_key_with_position;
 
 } DPDK_2.2;
+
+
+DPDK_18.08 {
+	global:
+
+	rte_hash_count;
+
+} DPDK_16.07;
diff --git a/test/test/test_hash.c b/test/test/test_hash.c
index edf41f5..b3db9fd 100644
--- a/test/test/test_hash.c
+++ b/test/test/test_hash.c
@@ -1103,6 +1103,7 @@  static int test_average_table_utilization(void)
 	unsigned i, j;
 	unsigned added_keys, average_keys_added = 0;
 	int ret;
+	unsigned int cnt;
 
 	printf("\n# Running test to determine average utilization"
 	       "\n  before adding elements begins to fail\n");
@@ -1121,13 +1122,24 @@  static int test_average_table_utilization(void)
 			for (i = 0; i < ut_params.key_len; i++)
 				simple_key[i] = rte_rand() % 255;
 			ret = rte_hash_add_key(handle, simple_key);
+			if (ret < 0)
+				break;
 		}
+
 		if (ret != -ENOSPC) {
 			printf("Unexpected error when adding keys\n");
 			rte_hash_free(handle);
 			return -1;
 		}
 
+		cnt = rte_hash_count(handle);
+		if (cnt != added_keys) {
+			printf("rte_hash_count returned wrong value %u, %u,"
+					"%u\n", j, added_keys, cnt);
+			rte_hash_free(handle);
+			return -1;
+		}
+
 		average_keys_added += added_keys;
 
 		/* Reset the table */
diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index ef5fce3..ae3ce3b 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -116,6 +116,7 @@  test_hash_multiwriter(void)
 
 	uint32_t duplicated_keys = 0;
 	uint32_t lost_keys = 0;
+	uint32_t count;
 
 	snprintf(name, 32, "test%u", calledCount++);
 	hash_params.name = name;
@@ -163,6 +164,14 @@  test_hash_multiwriter(void)
 				 NULL, CALL_MASTER);
 	rte_eal_mp_wait_lcore();
 
+
+	count = rte_hash_count(handle);
+	if (count != rounded_nb_total_tsx_insertion) {
+		printf("rte_hash_count returned wrong value %u, %d\n",
+				rounded_nb_total_tsx_insertion, count);
+		goto err3;
+	}
+
 	while (rte_hash_iterate(handle, &next_key, &next_data, &iter) >= 0) {
 		/* Search for the key in the list of keys added .*/
 		i = *(const uint32_t *)next_key;