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 A9B0AA00C3; Thu, 20 Jan 2022 09:21:43 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3A9F1411A4; Thu, 20 Jan 2022 09:21:43 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 183DB40042 for ; Thu, 20 Jan 2022 09:21:41 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v2 1/1] mempool: implement index-based per core cache X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Thu, 20 Jan 2022 09:21:40 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86E14@smartserver.smartshare.dk> In-Reply-To: <20220113053630.886638-2-dharmik.thakkar@arm.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v2 1/1] mempool: implement index-based per core cache Thread-Index: AdgIP5T2wpVhaWj1SB2Xtv+GgpMe2gFkz2qQ References: <20211224225923.806498-1-dharmik.thakkar@arm.com> <20220113053630.886638-1-dharmik.thakkar@arm.com> <20220113053630.886638-2-dharmik.thakkar@arm.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Dharmik Thakkar" , , "Olivier Matz" , "Andrew Rybchenko" Cc: , , , "Beilei Xing" 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 +CC Beilei as i40e maintainer > From: Dharmik Thakkar [mailto:dharmik.thakkar@arm.com] > Sent: Thursday, 13 January 2022 06.37 >=20 > Current mempool per core cache implementation stores pointers to mbufs > On 64b architectures, each pointer consumes 8B > This patch replaces it with index-based implementation, > where in each buffer is addressed by (pool base address + index) > It reduces the amount of memory/cache required for per core cache >=20 > L3Fwd performance testing reveals minor improvements in the cache > performance (L1 and L2 misses reduced by 0.60%) > with no change in throughput >=20 > Suggested-by: Honnappa Nagarahalli > Signed-off-by: Dharmik Thakkar > Reviewed-by: Ruifeng Wang > --- > lib/mempool/rte_mempool.h | 150 = +++++++++++++++++++++++++- > lib/mempool/rte_mempool_ops_default.c | 7 ++ > 2 files changed, 156 insertions(+), 1 deletion(-) >=20 > diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h > index 1e7a3c15273c..f2403fbc97a7 100644 > --- a/lib/mempool/rte_mempool.h > +++ b/lib/mempool/rte_mempool.h > @@ -50,6 +50,10 @@ > #include > #include >=20 > +#ifdef RTE_MEMPOOL_INDEX_BASED_LCORE_CACHE > +#include > +#endif > + > #include "rte_mempool_trace_fp.h" >=20 > #ifdef __cplusplus > @@ -239,6 +243,9 @@ struct rte_mempool { > int32_t ops_index; >=20 > struct rte_mempool_cache *local_cache; /**< Per-lcore local cache > */ > +#ifdef RTE_MEMPOOL_INDEX_BASED_LCORE_CACHE > + void *pool_base_value; /**< Base value to calculate indices */ > +#endif >=20 > uint32_t populated_size; /**< Number of populated > objects. */ > struct rte_mempool_objhdr_list elt_list; /**< List of objects in > pool */ > @@ -1314,7 +1321,22 @@ rte_mempool_cache_flush(struct = rte_mempool_cache > *cache, > if (cache =3D=3D NULL || cache->len =3D=3D 0) > return; > rte_mempool_trace_cache_flush(cache, mp); > + > +#ifdef RTE_MEMPOOL_INDEX_BASED_LCORE_CACHE > + unsigned int i; > + unsigned int cache_len =3D cache->len; > + void *obj_table[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; > + void *base_value =3D mp->pool_base_value; > + uint32_t *cache_objs =3D (uint32_t *) cache->objs; Hi Dharmik and Honnappa, The essence of this patch is based on recasting the type of the objs = field in the rte_mempool_cache structure from an array of pointers to an = array of uint32_t. However, this effectively breaks the ABI, because the rte_mempool_cache = structure is public and part of the API. Some drivers [1] even bypass the mempool API and access the = rte_mempool_cache structure directly, assuming that the objs array in = the cache is an array of pointers. So you cannot recast the fields in = the rte_mempool_cache structure the way this patch requires. Although I do consider bypassing an API's accessor functions "spaghetti = code", this driver's behavior is formally acceptable as long as the = rte_mempool_cache structure is not marked as internal. I really liked your idea of using indexes instead of pointers, so I'm = very sorry to shoot it down. :-( [1]: E.g. the Intel i40e PMD, = http://code.dpdk.org/dpdk/latest/source/drivers/net/i40e/i40e_rxtx_vec_av= x512.c#L25 -Morten