DPDK patches and discussions
 help / color / mirror / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: "Abdullah Ömer Yamaç" <aomeryamac@gmail.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [PATCH v5] lib/hash: add defer queue reclaim API
Date: Mon, 13 May 2024 05:24:45 +0000	[thread overview]
Message-ID: <278F162C-737E-4B26-81D3-7FF221E60BAE@arm.com> (raw)
In-Reply-To: <20240427195418.1034591-1-aomeryamac@gmail.com>

Hi Abdullah,
        This looks good, except for one comment inline.

> On Apr 27, 2024, at 2:54 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       | 90 ++++++++++++++++++++++++++++++++++++++
> lib/hash/rte_cuckoo_hash.c | 21 +++++++++
> lib/hash/rte_hash.h        | 25 +++++++++++
> lib/hash/version.map       |  7 +++
> 4 files changed, 143 insertions(+)
>
> diff --git a/app/test/test_hash.c b/app/test/test_hash.c
> index d586878a22..e763d0503f 100644
> --- a/app/test/test_hash.c
> +++ b/app/test/test_hash.c
> @@ -2183,6 +2183,93 @@ 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]);
Why do we need to lookup the entries?

> +
> + /* 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 +2348,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..52ccada12a 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -1588,6 +1588,27 @@ 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) {
> + 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);
> + 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..e1e0375fd5 100644
> --- a/lib/hash/rte_hash.h
> +++ b/lib/hash/rte_hash.h
> @@ -674,6 +674,31 @@ 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
>

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.

  parent reply	other threads:[~2024-05-13  5:24 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 15:33 [PATCH] lib/hash,lib/rcu: feature hidden key count in hash Abdullah Ömer Yamaç
2024-02-16 12:41 ` Thomas Monjalon
2024-02-19 17:36 ` Medvedkin, Vladimir
2024-02-19 21:28   ` Abdullah Ömer Yamaç
2024-02-19 23:35     ` Honnappa Nagarahalli
2024-02-20 18:58       ` Abdullah Ömer Yamaç
2024-02-21  3:23         ` Honnappa Nagarahalli
2024-02-21 21:51           ` Abdullah Ömer Yamaç
2024-02-22  2:37             ` Honnappa Nagarahalli
2024-02-22 12:39               ` Abdullah Ömer Yamaç
2024-02-22 16:44                 ` Honnappa Nagarahalli
2024-02-28 11:44                   ` Abdullah Ömer Yamaç
2024-02-28 14:51                     ` Honnappa Nagarahalli
2024-03-02 20:22                       ` Abdullah Ömer Yamaç
2024-03-02 21:08                         ` [PATCH v2] lib/hash: feature reclaim defer queue Abdullah Ömer Yamaç
2024-03-02 21:27                           ` Abdullah Ömer Yamaç
2024-03-03 19:14                             ` Honnappa Nagarahalli
2024-03-04  8:27                               ` Abdullah Ömer Yamaç
2024-03-04 21:58                                 ` Honnappa Nagarahalli
2024-03-06  8:55                                   ` [PATCH v3] lib/hash: add defer queue reclaim API Abdullah Ömer Yamaç
2024-03-06  8:59                                   ` Abdullah Ömer Yamaç
2024-03-06 10:13                                   ` Abdullah Ömer Yamaç
2024-03-14  7:04                                     ` Abdullah Ömer Yamaç
2024-04-04 10:11                                       ` Abdullah Ömer Yamaç
2024-04-05  2:11                                         ` Honnappa Nagarahalli
2024-04-15 11:26                                         ` [PATCH v4] " Abdullah Ömer Yamaç
2024-04-23 13:49                                           ` Abdullah Ömer Yamaç
2024-04-23 21:24                                           ` Stephen Hemminger
2024-04-25 14:03                                             ` Abdullah Ömer Yamaç
2024-04-27 19:54                                               ` [PATCH v5] " Abdullah Ömer Yamaç
2024-05-10  3:05                                                 ` Abdullah Ömer Yamaç
2024-05-13  5:24                                                 ` Honnappa Nagarahalli [this message]
2024-05-13 15:34                                                   ` Abdullah Ömer Yamaç
2024-05-15 10:49                                                     ` Abdullah Ömer Yamaç
2024-05-15 10:54                                                     ` [PATCH v6] " Abdullah Ömer Yamaç
2024-05-20 18:50                                                       ` Honnappa Nagarahalli
2024-06-18 14:09                                                         ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=278F162C-737E-4B26-81D3-7FF221E60BAE@arm.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=aomeryamac@gmail.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).