Re: [PATCH v2] lib/hash: feature reclaim defer queue

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

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

Just one more question.

On Sun, Mar 3, 2024 at 10:14 PM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:

> Hello Abdullah,
>         Thank you for the patch, few comments inline.
>
> The short commit log could be changed as follows:
>
> "lib/hash: add defer queue reclaim API”
>
> > On Mar 2, 2024, at 3:27 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>
> > Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Please add this only after you get an explicit Ack on the patch.
>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 23 +++++++++++++++++++++++
> > lib/hash/rte_hash.h        | 14 ++++++++++++++
> > lib/hash/version.map       |  7 +++++++
> > 3 files changed, 44 insertions(+)
> >
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 9cf94645f6..254fa80cc5 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)
> We need to add freed, pending and available parameters to this API. I
> think this information will be helpful for the users. For ex: in your use
> case, you could use the pending value to calculate the available hash
> entries.
>
> The second parameter, "Maximum number of resources to free.", should be
available also? I set this value to " h->hash_rcu_cfg->max_reclaim_size",
but it can be a parameter in addition to the above parameters

> > +{
> > + int ret;
> > +
> > + if (h->hash_rcu_cfg == NULL || h->dq == NULL) {
> We can skip NULL check for h->dq as the RCU reclaim API makes the same
> check.
>
> > + rte_errno = EINVAL;
> > + return -1;
> > + }
> > +
> > + ret = rte_rcu_qsbr_dq_reclaim(h->dq,
> h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL);
> > + 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..c119477d50 100644
> > --- a/lib/hash/rte_hash.h
> > +++ b/lib/hash/rte_hash.h
> > @@ -674,6 +674,21 @@ 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
> > + * @return
> > + *   On success - 0
> > + *   On error - 1 with error code set in rte_errno.
> > + *   Possible rte_errno codes are:
> > + *   - EINVAL - invalid pointer or invalid rcu mode
> We can remove the ‘invalid rcd mode’.
>
> > + */
> > +__rte_experimental
> > +int rte_hash_rcu_qsbr_dq_reclaim(struct rte_hash *h);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/hash/version.map b/lib/hash/version.map
> > index 6b2afebf6b..cec0e8fc67 100644
> > --- a/lib/hash/version.map
> > +++ b/lib/hash/version.map
> > @@ -48,3 +48,9 @@ DPDK_24 {
> >
> > local: *;
> > };
> > +
> > +EXPERIMENTAL {
> > + global:
> > +
> > + rte_hash_rcu_qsbr_dq_reclaim;
> > +}
> > \ No newline at end of file
> > --
> > 2.34.1
> >
>
>

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