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

["Abdullah Ömer Yamaç" <aomeryamac@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 8233 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 9860 bytes --]