[v4] lib/hash: add defer queue reclaim API

Message ID 20240415112602.690972-1-aomeryamac@gmail.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series [v4] lib/hash: add defer queue reclaim API |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing fail Testing issues
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Abdullah Ömer Yamaç April 15, 2024, 11:26 a.m. UTC
  This patch adds a new feature to the hash library to allow the user to
reclaim the defer queue. This is useful when the user wants to force
reclaim resources that are not being used. This API is only available
if the RCU is enabled.

Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
---
 app/test/test_hash.c       | 97 ++++++++++++++++++++++++++++++++++++++
 lib/hash/rte_cuckoo_hash.c | 23 +++++++++
 lib/hash/rte_hash.h        | 24 ++++++++++
 lib/hash/version.map       |  7 +++
 4 files changed, 151 insertions(+)
  

Comments

Abdullah Ömer Yamaç April 23, 2024, 1:49 p.m. UTC | #1
Hello, is there any other comment on this patch? Thanks

On Mon, Apr 15, 2024 at 2:26 PM Abdullah Ömer Yamaç <aomeryamac@gmail.com>
wrote:

> This patch adds a new feature to the hash library to allow the user to
> reclaim the defer queue. This is useful when the user wants to force
> reclaim resources that are not being used. This API is only available
> if the RCU is enabled.
>
> Signed-off-by: Abdullah Ömer Yamaç <aomeryamac@gmail.com>
> ---
>  app/test/test_hash.c       | 97 ++++++++++++++++++++++++++++++++++++++
>  lib/hash/rte_cuckoo_hash.c | 23 +++++++++
>  lib/hash/rte_hash.h        | 24 ++++++++++
>  lib/hash/version.map       |  7 +++
>  4 files changed, 151 insertions(+)
>
> diff --git a/app/test/test_hash.c b/app/test/test_hash.c
> index d586878a22..ebeda8c322 100644
> --- a/app/test/test_hash.c
> +++ b/app/test/test_hash.c
> @@ -2183,6 +2183,100 @@ test_hash_rcu_qsbr_sync_mode(uint8_t ext_bkt)
>
>  }
>
> +/*
> + * rte_hash_rcu_qsbr_dq_reclaim unit test.
> + */
> +static int
> +test_hash_rcu_qsbr_dq_reclaim(void)
> +{
> +       size_t sz;
> +       int32_t status;
> +       unsigned int total_entries = 8;
> +       unsigned int freed, pending, available;
> +       uint32_t reclaim_keys[8] = {10, 11, 12, 13, 14, 15, 16, 17};
> +       struct rte_hash_rcu_config rcu_cfg = {0};
> +       struct rte_hash_parameters hash_params = {
> +               .name = "test_hash_rcu_qsbr_dq_reclaim",
> +               .entries = total_entries,
> +               .key_len = sizeof(uint32_t),
> +               .hash_func = NULL,
> +               .hash_func_init_val = 0,
> +               .socket_id = 0,
> +       };
> +
> +       hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
> +
> +       g_qsv = NULL;
> +       g_handle = NULL;
> +
> +       printf("\n# Running RCU QSBR DQ mode, reclaim defer queue
> functional test\n");
> +
> +       g_handle = rte_hash_create(&hash_params);
> +       RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation failed");
> +
> +       /* Create RCU QSBR variable */
> +       sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
> +       g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
> +                                       RTE_CACHE_LINE_SIZE,
> SOCKET_ID_ANY);
> +       RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL,
> +                                                        "RCU QSBR
> variable creation failed");
> +
> +       status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
> +       RETURN_IF_ERROR_RCU_QSBR(status != 0,
> +                                                        "RCU QSBR
> variable initialization failed");
> +
> +       rcu_cfg.v = g_qsv;
> +       rcu_cfg.dq_size = total_entries;
> +       rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
> +
> +       /* Attach RCU QSBR to hash table */
> +       status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
> +       RETURN_IF_ERROR_RCU_QSBR(status != 0,
> +                                                        "Attach RCU QSBR
> to hash table failed");
> +
> +       /* Register pseudo reader */
> +       status = rte_rcu_qsbr_thread_register(g_qsv, 0);
> +       RETURN_IF_ERROR_RCU_QSBR(status != 0,
> +                                                        "RCU QSBR thread
> registration failed");
> +       rte_rcu_qsbr_thread_online(g_qsv, 0);
> +
> +       /* Fill half of the hash table */
> +       for (size_t i = 0; i < total_entries / 2; i++)
> +               status = rte_hash_add_key(g_handle, &reclaim_keys[i]);
> +
> +       /* Lookup inserted elements*/
> +       for (size_t i = 0; i < total_entries / 2; i++)
> +               rte_hash_lookup(g_handle, &reclaim_keys[i]);
> +
> +       /* Try to put these elements into the defer queue*/
> +       for (size_t i = 0; i < total_entries / 2; i++)
> +               rte_hash_del_key(g_handle, &reclaim_keys[i]);
> +
> +       /* Reader quiescent */
> +       rte_rcu_qsbr_quiescent(g_qsv, 0);
> +
> +       status = rte_hash_add_key(g_handle, &reclaim_keys[0]);
> +       RETURN_IF_ERROR_RCU_QSBR(status < 0,
> +                                                        "failed to add
> key (pos[%u]=%d)", 0,
> +                                                        status);
> +
> +       /* This should be (total_entries / 2) + 1 (last add) */
> +       unsigned int hash_size = rte_hash_count(g_handle);
> +
> +       /* Freed size should be (total_entries / 2) */
> +       rte_hash_rcu_qsbr_dq_reclaim(g_handle, &freed, &pending,
> &available);
> +
> +       rte_hash_free(g_handle);
> +       rte_free(g_qsv);
> +
> +       if (hash_size != (total_entries / 2 + 1) || freed !=
> (total_entries / 2)) {
> +               printf("Failed to reclaim defer queue\n");
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
>  /*
>   * Do all unit and performance tests.
>   */
> @@ -2261,6 +2355,9 @@ test_hash(void)
>         if (test_hash_rcu_qsbr_sync_mode(1) < 0)
>                 return -1;
>
> +       if (test_hash_rcu_qsbr_dq_reclaim() < 0)
> +               return -1;
> +
>         return 0;
>  }
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 9cf94645f6..4a44aadd9a 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,29 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
>         return 0;
>  }
>
> +int
> +rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> +                                       unsigned int *pending, unsigned
> int *available)
> +{
> +       int ret;
> +
> +       if (h == NULL || h->hash_rcu_cfg == NULL) {
> +               rte_errno = EINVAL;
> +               return 1;
> +       }
> +
> +       ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> +                                                                 freed,
> pending, available);
> +       if (ret != 0) {
> +               HASH_LOG(ERR,
> +                                "%s: could not reclaim the defer queue in
> hash table",
> +                                __func__);
> +               return 1;
> +       }
> +
> +       return 0;
> +}
> +
>  static inline void
>  remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
>                 unsigned int i)
> diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
> index 7ecc021111..edfa262aca 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,30 @@ rte_hash_iterate(const struct rte_hash *h, const void
> **key, void **data, uint32
>   */
>  int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config
> *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enabled.
> + *
> + * @param h
> + *   The hash object to reclaim resources.
> + * @param freed
> + *   Number of resources that were freed.
> + * @param pending
> + *   Number of resources pending on the defer queue.
> + *   This number might not be accurate if multi-thread safety is
> configured.
> + * @param available
> + *   Number of resources that can be added to the defer queue.
> + *   This number might not be accurate if multi-thread safety is
> configured.
> + * @return
> + *   On success - 0
> + *   On error - 1 with error code set in rte_errno.
> + *   Possible rte_errno codes are:
> + *   - EINVAL - invalid pointer
> + */
> +__rte_experimental
> +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> +               unsigned int *pending, unsigned int *available);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/hash/version.map b/lib/hash/version.map
> index 6f4bcdb71b..d348dd9196 100644
> --- a/lib/hash/version.map
> +++ b/lib/hash/version.map
> @@ -53,3 +53,10 @@ INTERNAL {
>         rte_thash_gfni_stub;
>         rte_thash_gfni_bulk_stub;
>  };
> +
> +EXPERIMENTAL {
> +       global:
> +
> +       # added in 24.07
> +       rte_hash_rcu_qsbr_dq_reclaim;
> +};
> --
> 2.34.1
>
>
  
Stephen Hemminger April 23, 2024, 9:24 p.m. UTC | #2
Unaddressed
On Mon, 15 Apr 2024 11:26:02 +0000
Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:

> +	ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
> +								  freed, pending, available);

Indention here is odd. I would expect "freed," to line up right under h->dq.
Since rte_rcu_qsbrs_dq_reclaim logs error on invalid parameters, this function should as well.

Total indent fixes:

diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 4a44aadd9a..e1ea810024 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1590,21 +1590,20 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
 
 int
 rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
-					unsigned int *pending, unsigned int *available)
+			     unsigned int *pending, unsigned int *available)
 {
 	int ret;
 
 	if (h == NULL || h->hash_rcu_cfg == NULL) {
+		HASH_LOG(ERR, "Invalid input parameter");
 		rte_errno = EINVAL;
 		return 1;
 	}
 
 	ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
-								  freed, pending, available);
+				      freed, pending, available);
 	if (ret != 0) {
-		HASH_LOG(ERR,
-				 "%s: could not reclaim the defer queue in hash table",
-				 __func__);
+		HASH_LOG(ERR, "%s: could not reclaim the defer queue in hash table", __func__);
 		return 1;
 	}
  
Abdullah Ömer Yamaç April 25, 2024, 2:03 p.m. UTC | #3
Thanks for the comments. This is due to the tab size, and I will fix them.

On Wed, Apr 24, 2024 at 12:24 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Mon, 15 Apr 2024 11:26:02 +0000
> Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> > +     ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> > +                                                               freed,
> pending, available);
>
> Indention here is odd. I would expect "freed," to line up right under
> h->dq.
> Since rte_rcu_qsbrs_dq_reclaim logs error on invalid parameters, this
> function should as well.
>
> Total indent fixes:
>
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 4a44aadd9a..e1ea810024 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1590,21 +1590,20 @@ rte_hash_rcu_qsbr_add(struct rte_hash *h, struct
> rte_hash_rcu_config *cfg)
>
>  int
>  rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
> -                                       unsigned int *pending, unsigned
> int *available)
> +                            unsigned int *pending, unsigned int
> *available)
>  {
>         int ret;
>
>         if (h == NULL || h->hash_rcu_cfg == NULL) {
> +               HASH_LOG(ERR, "Invalid input parameter");
>                 rte_errno = EINVAL;
>                 return 1;
>         }
>
>         ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size,
> -                                                                 freed,
> pending, available);
> +                                     freed, pending, available);
>         if (ret != 0) {
> -               HASH_LOG(ERR,
> -                                "%s: could not reclaim the defer queue in
> hash table",
> -                                __func__);
> +               HASH_LOG(ERR, "%s: could not reclaim the defer queue in
> hash table", __func__);
>                 return 1;
>         }
>
>
  

Patch

diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index d586878a22..ebeda8c322 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -2183,6 +2183,100 @@  test_hash_rcu_qsbr_sync_mode(uint8_t ext_bkt)
 
 }
 
+/*
+ * rte_hash_rcu_qsbr_dq_reclaim unit test.
+ */
+static int
+test_hash_rcu_qsbr_dq_reclaim(void)
+{
+	size_t sz;
+	int32_t status;
+	unsigned int total_entries = 8;
+	unsigned int freed, pending, available;
+	uint32_t reclaim_keys[8] = {10, 11, 12, 13, 14, 15, 16, 17};
+	struct rte_hash_rcu_config rcu_cfg = {0};
+	struct rte_hash_parameters hash_params = {
+		.name = "test_hash_rcu_qsbr_dq_reclaim",
+		.entries = total_entries,
+		.key_len = sizeof(uint32_t),
+		.hash_func = NULL,
+		.hash_func_init_val = 0,
+		.socket_id = 0,
+	};
+
+	hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
+
+	g_qsv = NULL;
+	g_handle = NULL;
+
+	printf("\n# Running RCU QSBR DQ mode, reclaim defer queue functional test\n");
+
+	g_handle = rte_hash_create(&hash_params);
+	RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation failed");
+
+	/* Create RCU QSBR variable */
+	sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
+	g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
+					RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY);
+	RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL,
+							 "RCU QSBR variable creation failed");
+
+	status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
+	RETURN_IF_ERROR_RCU_QSBR(status != 0,
+							 "RCU QSBR variable initialization failed");
+
+	rcu_cfg.v = g_qsv;
+	rcu_cfg.dq_size = total_entries;
+	rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
+
+	/* Attach RCU QSBR to hash table */
+	status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
+	RETURN_IF_ERROR_RCU_QSBR(status != 0,
+							 "Attach RCU QSBR to hash table failed");
+
+	/* Register pseudo reader */
+	status = rte_rcu_qsbr_thread_register(g_qsv, 0);
+	RETURN_IF_ERROR_RCU_QSBR(status != 0,
+							 "RCU QSBR thread registration failed");
+	rte_rcu_qsbr_thread_online(g_qsv, 0);
+
+	/* Fill half of the hash table */
+	for (size_t i = 0; i < total_entries / 2; i++)
+		status = rte_hash_add_key(g_handle, &reclaim_keys[i]);
+
+	/* Lookup inserted elements*/
+	for (size_t i = 0; i < total_entries / 2; i++)
+		rte_hash_lookup(g_handle, &reclaim_keys[i]);
+
+	/* Try to put these elements into the defer queue*/
+	for (size_t i = 0; i < total_entries / 2; i++)
+		rte_hash_del_key(g_handle, &reclaim_keys[i]);
+
+	/* Reader quiescent */
+	rte_rcu_qsbr_quiescent(g_qsv, 0);
+
+	status = rte_hash_add_key(g_handle, &reclaim_keys[0]);
+	RETURN_IF_ERROR_RCU_QSBR(status < 0,
+							 "failed to add key (pos[%u]=%d)", 0,
+							 status);
+
+	/* This should be (total_entries / 2) + 1 (last add) */
+	unsigned int hash_size = rte_hash_count(g_handle);
+
+	/* Freed size should be (total_entries / 2) */
+	rte_hash_rcu_qsbr_dq_reclaim(g_handle, &freed, &pending, &available);
+
+	rte_hash_free(g_handle);
+	rte_free(g_qsv);
+
+	if (hash_size != (total_entries / 2 + 1) || freed != (total_entries / 2)) {
+		printf("Failed to reclaim defer queue\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 /*
  * Do all unit and performance tests.
  */
@@ -2261,6 +2355,9 @@  test_hash(void)
 	if (test_hash_rcu_qsbr_sync_mode(1) < 0)
 		return -1;
 
+	if (test_hash_rcu_qsbr_dq_reclaim() < 0)
+		return -1;
+
 	return 0;
 }
 
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 9cf94645f6..4a44aadd9a 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -1588,6 +1588,29 @@  rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg)
 	return 0;
 }
 
+int
+rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+					unsigned int *pending, unsigned int *available)
+{
+	int ret;
+
+	if (h == NULL || h->hash_rcu_cfg == NULL) {
+		rte_errno = EINVAL;
+		return 1;
+	}
+
+	ret = rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max_reclaim_size,
+								  freed, pending, available);
+	if (ret != 0) {
+		HASH_LOG(ERR,
+				 "%s: could not reclaim the defer queue in hash table",
+				 __func__);
+		return 1;
+	}
+
+	return 0;
+}
+
 static inline void
 remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt,
 		unsigned int i)
diff --git a/lib/hash/rte_hash.h b/lib/hash/rte_hash.h
index 7ecc021111..edfa262aca 100644
--- a/lib/hash/rte_hash.h
+++ b/lib/hash/rte_hash.h
@@ -674,6 +674,30 @@  rte_hash_iterate(const struct rte_hash *h, const void **key, void **data, uint32
  */
 int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_config *cfg);
 
+/**
+ * Reclaim resources from the defer queue.
+ * This API reclaim the resources from the defer queue if rcu is enabled.
+ *
+ * @param h
+ *   The hash object to reclaim resources.
+ * @param freed
+ *   Number of resources that were freed.
+ * @param pending
+ *   Number of resources pending on the defer queue.
+ *   This number might not be accurate if multi-thread safety is configured.
+ * @param available
+ *   Number of resources that can be added to the defer queue.
+ *   This number might not be accurate if multi-thread safety is configured.
+ * @return
+ *   On success - 0
+ *   On error - 1 with error code set in rte_errno.
+ *   Possible rte_errno codes are:
+ *   - EINVAL - invalid pointer
+ */
+__rte_experimental
+int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h, unsigned int *freed,
+		unsigned int *pending, unsigned int *available);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/hash/version.map b/lib/hash/version.map
index 6f4bcdb71b..d348dd9196 100644
--- a/lib/hash/version.map
+++ b/lib/hash/version.map
@@ -53,3 +53,10 @@  INTERNAL {
 	rte_thash_gfni_stub;
 	rte_thash_gfni_bulk_stub;
 };
+
+EXPERIMENTAL {
+	global:
+
+	# added in 24.07
+	rte_hash_rcu_qsbr_dq_reclaim;
+};