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 DEA9C43A26; Wed, 31 Jan 2024 23:39:58 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79FD7402C5; Wed, 31 Jan 2024 23:39:58 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 859BD4026C for ; Wed, 31 Jan 2024 23:39:56 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 442402079B; Wed, 31 Jan 2024 23:39:56 +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] mbuf: replace GCC marker extension with C11 anonymous unions X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 31 Jan 2024 23:39:52 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F1D7@smartserver.smartshare.dk> In-Reply-To: <20240131210921.GD11576@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] mbuf: replace GCC marker extension with C11 anonymous unions Thread-Index: AdpUicVV6732fXJ1RX6+Vlny5V1NbAACO6bw References: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com> <1706657173-26166-2-git-send-email-roretzla@linux.microsoft.com> <98CBD80474FA8B44BF855DF32C47DC35E9F1D0@smartserver.smartshare.dk> <20240131210921.GD11576@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" Cc: , "Andrew Boyer" , "Andrew Rybchenko" , "Bruce Richardson" , "Chenbo Xia" , "Konstantin Ananyev" , "Maxime Coquelin" 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: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Wednesday, 31 January 2024 22.09 >=20 > On Wed, Jan 31, 2024 at 10:18:37AM +0100, Morten Br=F8rup wrote: > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > > > Sent: Wednesday, 31 January 2024 00.26 > > > [...] > > > struct rte_mempool *pool; /**< Pool from which mbuf was > > > allocated. */ > > > > > > /* second cache line - fields only used in slow path or on TX */ > > > - RTE_MARKER cacheline1 __rte_cache_min_aligned; > > > + union { > > > + void *cacheline1; > > > > The __rte_cache_min_aligned cannot be removed. It provides cache = line > alignment for 32 bit platforms, where pointers in the first cache line > only use 4 byte. >=20 > oh no i forgot i needed to figure this out before submission. now that > it's here though i could use some help / suggestions. >=20 > the existing __rte_cache_min_aligned (and indeed standard alignas) > facilities are not of great utility when applied to anonymous unions, > further complicating things is that it also has to work with C++. >=20 > i'll take this away and work on it some more but does anyone here have > a > suggestion on how to align this anonymous union data member to the > desired alignment *without* the union being padded to min cache line > size and as a consequence causing the rte_mbuf struct to be 3 instead > of 2 cache lines? (that's essentially the problem i need help = solving). I would suggest to simply remove __rte_cache_min_aligned (and the = implicit padding that comes with it), and instead conditionally (#ifdef = RTE_ARCH_32) add an explicit uintptr_t padding field after each pointer = field in the rte_mbuf struct's first cache line. But that would break the 32-bit ABI, so instead insert the explicit = padding in the rte_mbuf struct at the end of its first cache line (where = the implicit padding from __rte_cache_min_aligned is currently added), = something like this: struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ +#ifdef RTE_ARCH_32 + /* Padding to ensure correct alignment of cacheline1. */ + uintptr_t pad_buf_addr; +#if !RTE_IOVA_IN_MBUF + uintptr_t pad_next; +#endif +#endif /* RTE_ARCH_32 */ /* second cache line - fields only used in slow path or on TX */ To be on the safe side, add a static_assert to verify that = offsetof(struct rte_mbuf, cacheline1) =3D=3D RTE_CACHE_LINE_MIN_SIZE. = This should be true for all architectures, i.e. both 64 bit and 32 bit.