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 8C94B43B37; Wed, 14 Feb 2024 15:28:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5ADD54026A; Wed, 14 Feb 2024 15:28:26 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 3973A40263 for ; Wed, 14 Feb 2024 15:28:24 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 3601220B2000; Wed, 14 Feb 2024 06:28:23 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3601220B2000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1707920903; bh=clZatnDFjaCbab1MRCRqNP5Fp72+8yFMwSVim4SASMM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CS6WSZyvDxDsAMB7TQSna3LXGZYAySJlHREFacuuE8R7AgtUduT1fuDsYrvA2hLU6 vT8PHA6Icv22K18MHIVCX/ghuZGAqfp8yhIrzNQGRMqu6n5Z1kwd9kV54EDFuELsfl +o7q4FkGCc4KsbgxY4fTK3FxBRCQJQWN6paW7drk= Date: Wed, 14 Feb 2024 06:28:23 -0800 From: Tyler Retzlaff To: David Marchand Cc: dev@dpdk.org, Andrew Rybchenko , Bruce Richardson , Chengwen Feng , Cristian Dumitrescu , David Christensen , David Hunt , Ferruh Yigit , Honnappa Nagarahalli , Jasvinder Singh , Jerin Jacob , Kevin Laatz , Konstantin Ananyev , Min Zhou , Ruifeng Wang , Sameh Gobriel , Stanislaw Kardach , Thomas Monjalon , Vladimir Medvedkin , Yipeng Wang Subject: Re: [PATCH v3 08/39] mbuf: remove unnecessary explicit alignment Message-ID: <20240214142823.GA28549@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1707873986-29352-1-git-send-email-roretzla@linux.microsoft.com> <1707894382-307-1-git-send-email-roretzla@linux.microsoft.com> <1707894382-307-9-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Wed, Feb 14, 2024 at 02:12:17PM +0100, David Marchand wrote: > On Wed, Feb 14, 2024 at 8:07 AM Tyler Retzlaff > wrote: > > > > Remove explicit alignment with __rte_aligned(sizeof(T)) on buf_iova > > field in the absence of packing the field should be correctly aligned. > > > > Signed-off-by: Tyler Retzlaff > > --- > > lib/mbuf/rte_mbuf_core.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h > > index 5688683..7369e3e 100644 > > --- a/lib/mbuf/rte_mbuf_core.h > > +++ b/lib/mbuf/rte_mbuf_core.h > > @@ -463,7 +463,7 @@ enum { > > /** > > * The generic rte_mbuf, containing a packet mbuf. > > */ > > -struct rte_mbuf { > > +struct __rte_cache_aligned rte_mbuf { > > RTE_MARKER cacheline0; > > > > void *buf_addr; /**< Virtual address of segment buffer. */ > > @@ -476,7 +476,7 @@ struct rte_mbuf { > > * 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_iova_t buf_iova; > > #else > > Before the change, for 32bits build: > > struct rte_mbuf { > RTE_MARKER cacheline0; /* 0 0 */ > void * buf_addr; /* 0 4 */ > > /* XXX 4 bytes hole, try to pack */ > > rte_iova_t buf_iova > __attribute__((__aligned__(8))); /* 8 8 */ > RTE_MARKER64 rearm_data; /* 16 0 */ > ... > > After the change: > > struct rte_mbuf { > RTE_MARKER cacheline0; /* 0 0 */ > void * buf_addr; /* 0 4 */ > rte_iova_t buf_iova; /* 4 8 */ > RTE_MARKER64 rearm_data; /* 12 0 */ > ... > > So it looks like uint64_t is not naturally aligned on 8 bytes for x86 > 32 bits, which explains the current explicit constraint (and comment > in the header). How I love x86, I forgot that uint64_t can be 4 byte aligned on x86. I've done this in 3 places I will restore them all. I wonder why my test builds didn't build bug out (something separate I better investigate). Thanks for calling it out! > See also 586ec205bcbb ("mbuf: fix 64-bit address alignment in 32-bit builds"). > > > This results in a cascading offset changes triggering multiple build > errors in vectorised code: > > In file included from > ../../../git/pub/dpdk.org/main/lib/eal/x86/include/rte_vect.h:16, > from > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c:5: > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c: > In function ‘idpf_singleq_rearm_common’: > ../../../git/pub/dpdk.org/main/lib/eal/include/rte_common.h:509:55: > error: size of unnamed array is negative > 509 | #define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - > 2*!!(condition)])) > | ^ > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c:68:17: > note: in expansion of macro ‘RTE_BUILD_BUG_ON’ > 68 | RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, buf_iova) != > | ^~~~~~~~~~~~~~~~ > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c: > In function ‘_idpf_singleq_recv_raw_pkts_avx512’: > ../../../git/pub/dpdk.org/main/lib/eal/include/rte_common.h:509:55: > error: size of unnamed array is negative > 509 | #define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - > 2*!!(condition)])) > | ^ > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c:461:17: > note: in expansion of macro ‘RTE_BUILD_BUG_ON’ > 461 | RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, > rearm_data) != > | ^~~~~~~~~~~~~~~~ > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c: > In function ‘_idpf_splitq_recv_raw_pkts_avx512’: > ../../../git/pub/dpdk.org/main/lib/eal/include/rte_common.h:509:55: > error: size of unnamed array is negative > 509 | #define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - > 2*!!(condition)])) > | ^ > ../../../git/pub/dpdk.org/main/drivers/common/idpf/idpf_common_rxtx_avx512.c:921:17: > note: in expansion of macro ‘RTE_BUILD_BUG_ON’ > 921 | RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, > rearm_data) != > | ^~~~~~~~~~~~~~~~ > > > -- > David Marchand