From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9C89C43B91; Mon, 4 Mar 2024 09:27:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 49C6A4026B; Mon, 4 Mar 2024 09:27:39 +0100 (CET) Received: from mail-oa1-f51.google.com (mail-oa1-f51.google.com [209.85.160.51]) by mails.dpdk.org (Postfix) with ESMTP id 14A0A40041 for ; Mon, 4 Mar 2024 09:27:38 +0100 (CET) Received: by mail-oa1-f51.google.com with SMTP id 586e51a60fabf-21f70f72fb5so3199636fac.1 for ; Mon, 04 Mar 2024 00:27:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709540857; x=1710145657; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=JL0VekMk0dwjBPNKeN6IBY2SL099Vnk4vYRTQ6udhu8=; b=QyqHKThrqybs7ga4wuC/L/tvAV0ftwfVlvMsHFiHdAROPyCmZDrG/IPpn5XxeeaWou /bEeQvj/nqRpIwhr1uzV+ciWnIcoMMiyps5wEhvi06t46lfO1e395vZR+vVVmUakXPBD t6aZkBHCv9qHTby4gwsEVrsJKS0zZ4/FWWP6hmcAzGpCqb9sTqEmM5MuiZCcylqi7Q5+ EGfJfW3Dotv+t8V8cbDwIb+O2PMg29El5nhCla7N/b01bctOuSJ4bByUPIQtw/JTGhnf iF8vxsBQfXOJbSKbXOCNwBLU0uRDtBci2zCH1wQcVrsha+rPS8N2zfYF1EmTvBT3PqAh uBSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709540857; x=1710145657; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JL0VekMk0dwjBPNKeN6IBY2SL099Vnk4vYRTQ6udhu8=; b=KQ7S1XuV/9AtXaBy244ZpG1Mi4Gtn+4RSaGzz2JWzOUBOloFAkKvlTd3H3fAYL2BMd ny5n97HoRcI3vs9THNzqRAiO0IrTAee5+5Bll78YSeeDCzmmdHATedkR0RJCjA0ujl5y B5MQBAdR3V6DPaRge2d95ASpL2E5BHXchiKTni2l6GVFZIu+f9QADBV4ub2fjWqAyNl2 uT5LjDAjF1l2K54XjaAfAiQzzEAC1LxIIV7rZ97AeGfnVHSrOCWCEzsQNAcm4Oy9iSuE KXgIYhiGbS6Mly9lMu3QR9/3P19jf4brN+uzTGj4KIDknCXAlAomNnd6Lk+wwT8dvwI7 84yQ== X-Gm-Message-State: AOJu0YzLG+OEvFYWyK6gK6co6VKctJGDjMzotERV1kqMZwZyiW9pwHA2 sDjykxQZC4xRFIIG/2r9JKlP6J5CknZ0B4LctFMDyuF8C7Kg/wbgXGUl9FBQJzTnHSMQGUiDwfP 46n5WU/RrBo+u9M0ZBRy9NdNZgEk= X-Google-Smtp-Source: AGHT+IHwg5IIlJypEZAgoyIaqEF55TIdkJPRONmTeTwlE+jM11HHshoJWcdHWUKE3Zn0MT5HmZ4kHStG5zKY/aT3CL4= X-Received: by 2002:a05:6870:f818:b0:21e:7156:a69f with SMTP id fr24-20020a056870f81800b0021e7156a69fmr10189195oab.42.1709540857193; Mon, 04 Mar 2024 00:27:37 -0800 (PST) MIME-Version: 1.0 References: <20240302210822.202270-1-aomeryamac@gmail.com> <20240302212711.204396-1-aomeryamac@gmail.com> <68684A5E-2746-4789-AA6C-7FF09FDC785B@arm.com> In-Reply-To: <68684A5E-2746-4789-AA6C-7FF09FDC785B@arm.com> From: =?UTF-8?B?QWJkdWxsYWggw5ZtZXIgWWFtYcOn?= Date: Mon, 4 Mar 2024 11:27:28 +0300 Message-ID: Subject: Re: [PATCH v2] lib/hash: feature reclaim defer queue To: Honnappa Nagarahalli Cc: "dev@dpdk.org" , nd Content-Type: multipart/alternative; boundary="00000000000085aa7d0612d17f89" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --00000000000085aa7d0612d17f89 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Just one more question. On Sun, Mar 3, 2024 at 10:14=E2=80=AFPM 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=E2=80=9D > > > On Mar 2, 2024, at 3:27=E2=80=AFPM, Abdullah =C3=96mer Yama=C3=A7 > 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 =C3=96mer Yama=C3=A7 > > Acked-by: Honnappa Nagarahalli > 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 =3D=3D NULL || h->dq =3D=3D NULL) { > We can skip NULL check for h->dq as the RCU reclaim API makes the same > check. > > > + rte_errno =3D EINVAL; > > + return -1; > > + } > > + > > + ret =3D rte_rcu_qsbr_dq_reclaim(h->dq, > h->hash_rcu_cfg->max_reclaim_size, NULL, NULL, NULL); > > + if (ret !=3D 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_confi= g > *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 =E2=80=98invalid rcd mode=E2=80=99. > > > + */ > > +__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 > > > > --00000000000085aa7d0612d17f89 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Just one more question.

On Sun, Mar 3, 202= 4 at 10:14=E2=80=AFPM Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
Hello Abdullah,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 Thank you for the patch, few comments inline.
The short commit log could be changed as follows:

"lib/hash: add defer queue reclaim API=E2=80=9D

> On Mar 2, 2024, at 3:27=E2=80=AFPM, Abdullah =C3=96mer Yama=C3=A7 <= 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<= br> > if the RCU is enabled.
>
> Signed-off-by: Abdullah =C3=96mer Yama=C3=A7 <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=C2=A0 =C2=A0 =C2=A0 =C2=A0 | 14 ++++++++++++++
> lib/hash/version.map=C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 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, struc= t 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 resourc= es to free.", should be available also? I set this value to " h-&= gt;hash_rcu_cfg->max_reclaim_size", but it can be a parameter in ad= dition to the above parameters
> +{
> + int ret;
> +
> + if (h->hash_rcu_cfg =3D=3D NULL || h->dq =3D=3D NULL) {
We can skip NULL check for h->dq as the RCU reclaim API makes the same c= heck.

> + rte_errno =3D EINVAL;
> + return -1;
> + }
> +
> + ret =3D rte_rcu_qsbr_dq_reclaim(h->dq, h->hash_rcu_cfg->max= _reclaim_size, NULL, NULL, NULL);
> + if (ret !=3D 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
>=C2=A0 */
> int rte_hash_rcu_qsbr_add(struct rte_hash *h, struct rte_hash_rcu_conf= ig *cfg);
>
> +/**
> + * Reclaim resources from the defer queue.
> + * This API reclaim the resources from the defer queue if rcu is enab= led.
> + *
> + * @param h
> + *=C2=A0 =C2=A0the hash object to reclaim resources
> + * @return
> + *=C2=A0 =C2=A0On success - 0
> + *=C2=A0 =C2=A0On error - 1 with error code set in rte_errno.
> + *=C2=A0 =C2=A0Possible rte_errno codes are:
> + *=C2=A0 =C2=A0- EINVAL - invalid pointer or invalid rcu mode
We can remove the =E2=80=98invalid rcd mode=E2=80=99.

> + */
> +__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
>

--00000000000085aa7d0612d17f89--