> On Mar 4, 2024, at 2:27 AM, Abdullah Ömer Yamaç <aomerya...@gmail.com> wrote:
>
> Just one more question.
>
> On Sun, Mar 3, 2024 at 10:14 PM Honnappa Nagarahalli
> <honnappa.nagaraha...@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ç <aomerya...@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ç <aomerya...@gmail.com>
> > Acked-by: Honnappa Nagarahalli <honnappa.nagaraha...@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
We don’t have to expose that in the API. The user has provided that already in
max_reclaim_size. So, using "h->hash_rcu_cfg->max_reclaim_size” is enough.
> > +{
> > + 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
> >
>
