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 1843DA0542; Mon, 29 Aug 2022 20:32:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0820E41140; Mon, 29 Aug 2022 20:32:23 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 3D6114069D for ; Mon, 29 Aug 2022 20:32:22 +0200 (CEST) Content-class: urn:content-classes:message Subject: RE: [PATCH v1 2/4] mbuf: add second dynamic field member for VA only build MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Mon, 29 Aug 2022 20:32:20 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D872D1@smartserver.smartshare.dk> In-Reply-To: <20220829151626.2101336-3-sthotton@marvell.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v1 2/4] mbuf: add second dynamic field member for VA only build Thread-Index: Adi7uoCzvKzXf8xSTiuQiMewlT2F9wAGVhdw References: <20220829151626.2101336-3-sthotton@marvell.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Shijith Thotton" , Cc: , , , , , , 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 > From: Shijith Thotton [mailto:sthotton@marvell.com] > Sent: Monday, 29 August 2022 17.16 >=20 > mbuf physical address field is not used in builds which only uses VA. > It is used to expand the dynamic field area. >=20 > Signed-off-by: Shijith Thotton > --- > lib/mbuf/rte_mbuf_core.h | 26 +++++++++++++++++--------- > lib/mbuf/rte_mbuf_dyn.c | 2 ++ > 2 files changed, 19 insertions(+), 9 deletions(-) >=20 > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h > index 81cb07c2e4..98ce62fd6a 100644 > --- a/lib/mbuf/rte_mbuf_core.h > +++ b/lib/mbuf/rte_mbuf_core.h > @@ -579,15 +579,23 @@ struct rte_mbuf { > RTE_MARKER cacheline0; >=20 > void *buf_addr; /**< Virtual address of segment buffer. > */ > - /** > - * Physical address of segment buffer. > - * This field is invalid if the build is configured to use only > - * virtual address as IOVA (i.e. RTE_IOVA_AS_VA is defined). > - * Force alignment to 8-bytes, so as to ensure we have the exact > - * same mbuf cacheline0 layout for 32-bit and 64-bit. This makes > - * working on vector drivers easier. > - */ > - rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t)); > + RTE_STD_C11 > + union { > + /** > + * Physical address of segment buffer. > + * This field is invalid if the build is configured to use > only > + * virtual address as IOVA (i.e. RTE_IOVA_AS_VA is > defined). > + * Force alignment to 8-bytes, so as to ensure we have the > exact > + * same mbuf cacheline0 layout for 32-bit and 64-bit. This > makes > + * working on vector drivers easier. > + */ > + rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t)); > + /** > + * Reserved for dynamic field in builds where physical > address > + * field is invalid. > + */ > + uint64_t dynfield2; > + }; >=20 > /* next 8 bytes are initialised on RX descriptor rearm */ > RTE_MARKER64 rearm_data; I know that the intention here is to keep the rte_mbuf structure intact, = which will certainly improve the probability of getting this patch = series into DPDK. So, I will add a comment for the benefit of the other participants in = the discussion: With this patch, and in RTE_IOVA_AS_VA mode, it becomes possible to move = m->next into the first cache line, so rte_pktmbuf_prefree_seg() does not = have to touch the second cache line, thus potentially improving = performance by eliminating one cache miss per freed packet segment. (I = also recall someone mentioning that some PMDs set m->next on RX... If = that is the case, a cache miss per packet might also be avoidable in = those PMDs.) Obviously, moving m->next to the first cache line is not related to this = patch series, but would belong in a completely different patch.