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 64A8C43D27; Sat, 23 Mar 2024 00:41:59 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3C2D542EE2; Sat, 23 Mar 2024 00:41:59 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id CC962402BD for ; Sat, 23 Mar 2024 00:41:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711150917; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kFrF68lD6UBnl91LABOskobhlaSK61S4ikUNY5m5NMg=; b=M1nJG6XdNE2U9ngAWmkkedEy8gbrrb6fmIByRlLDG0jSWMdeaZN+k1pXwiGp3F5xBdL2r2 7gWxyqwQHo9kPcVTm9nNewjVj+1AiuMSUFlRSdHmHyMi9ZvQe/pUjazseRp/BY6I4KMnOV 9FbQjeQcBYEnw9flmgFNNkNR7lOLiiE= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-326-0REJJERiMG651AjVAcwSmg-1; Fri, 22 Mar 2024 19:41:54 -0400 X-MC-Unique: 0REJJERiMG651AjVAcwSmg-1 Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2d48d13b3c9so24322931fa.1 for ; Fri, 22 Mar 2024 16:41:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711150913; x=1711755713; h=in-reply-to:references:user-agent:subject:to:from:cc:message-id :date:content-transfer-encoding:mime-version:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=yZVc3d3Zhmb+jOE4Rr8vZvnNTp3ag/1ys+YkXWFC9Xo=; b=be5FwlUjkH1IH2orD77ldJQaKDm47IK8/vOuS7SZkfo9iPNY1YWNJqG6xDxjJJCwBx VLT2/biw3l9WF4y1Q8rX++ZrB5TmDlpkQilkVnXKsaaW4XpFmYn24LuYWxBfNpy00yYQ 77sGrdylBKvVaPxOWuHb3CQI/bvMjuyS5Y7bwVfiU9z0+HHXjnNKHsCzTg1A/4NgBp8/ uvdTxdqD3eyHecoJrqsgdqlAeXOzjAuxdvTL7CQ96xNwvudIl0yEEL/6ihC33y9XDQeB jC/L+du7HrTSOg/f1yqSbLXUUtSVComljNDjJzJupLaA4uGZ0aXwiaf3GpjSfxuWtTE4 qj2w== X-Gm-Message-State: AOJu0YxRcDzXdxUBfjhw+SCgeTTwMCVnaO4bummCtBit2vUF8d3uitU/ CBWguOXoeiJYQ/+MFiT8+mj1zCPGIQLvB6rE5aVGivRN3WbKoKizdLvYsk58XuMeEHjgEamowr/ E/jzH78Fdw0EII6UE5hGNHMjfNdJ73i3+dxRK4Hgk X-Received: by 2002:a2e:a277:0:b0:2d4:ae2f:cdc with SMTP id k23-20020a2ea277000000b002d4ae2f0cdcmr526692ljm.50.1711150913482; Fri, 22 Mar 2024 16:41:53 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGEMfoOqjEKbJee339RG8+/SZ3p+5oOX2FS6M1l+bRAo5k65464FPUxndHALB6c/qEe9bbmhA== X-Received: by 2002:a2e:a277:0:b0:2d4:ae2f:cdc with SMTP id k23-20020a2ea277000000b002d4ae2f0cdcmr526687ljm.50.1711150913057; Fri, 22 Mar 2024 16:41:53 -0700 (PDT) Received: from localhost (2a01cb000f8b9700ffd8872f0c4ad9d4.ipv6.abo.wanadoo.fr. [2a01:cb00:f8b:9700:ffd8:872f:c4a:d9d4]) by smtp.gmail.com with ESMTPSA id be3-20020a05600c1e8300b004140e701884sm865773wmb.22.2024.03.22.16.41.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 22 Mar 2024 16:41:52 -0700 (PDT) Mime-Version: 1.0 Date: Sat, 23 Mar 2024 00:41:51 +0100 Message-Id: Cc: , "Jerin Jacob" , "Kiran Kumar K" , "Nithin Dabilpuram" , "Zhirun Yan" From: "Robin Jarry" To: "Tyler Retzlaff" Subject: Re: [PATCH v2] graph: expose node context as pointers User-Agent: aerc/0.17.0-81-gd2764918eb55 References: <20240322163130.671185-2-rjarry@redhat.com> <20240322165615.GA31848@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20240322165615.GA31848@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8; format=Flowed 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 Hi Tyler, Tyler Retzlaff, Mar 22, 2024 at 17:56: > i can answer this! > > windows toolchains will only require __extension__ qualification on use > of statement expressions, so msvc won't require any use of __extension__ > in this patch. > > as a general rule of thumb __extension__ is something you may choose to > use for any gcc compiled code that is an extension to standard C and you > intend to use the -pedantic flag (i.e. -std=3Dc11 && -pedantic used toget= her) Got it, thanks! > > =09/* Fast path area */ > > #define RTE_NODE_CTX_SZ 16 > > -=09alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< Nod= e Context. */ > > +=09__extension__ alignas(RTE_CACHE_LINE_SIZE) union { > > __extension__ should not be on the anonymous union (since they are standa= rd C11). > > anonymous union declaration is actually a type with no name and then a da= ta > field of that type so __rte_aligned is most likely what you want, since > you're using RTE_CACHE_LINE_SIZE we can use __rte_cache_aligned. > > union __rte_cache_aligned { > ... your union fields ... > }; > > and i think checkpatches still gives a warning unrelated to alignment > for this but it can be safely ignored. it's the warning about alignment > that we care about and should be fixed. This passes the C++ header check but it breaks the static_assert I just=20 added. I believe the alignment is somehow transferred to all union=20 fields. And since ctx is an array, it makes the whole union . So before my patch: /* --- cacheline 3 boundary (192 bytes) --- */ uint8_t ctx[16] __attribute__((__aligned__(64))); /* 192 16 */ uint16_t size; /* 208 2 */ With the anonymous union aligned: /* --- cacheline 3 boundary (192 bytes) --- */ union { uint8_t ctx[16]; /* 192 16 */ struct { void * ctx_ptr; /* 192 8 */ void * ctx_ptr2; /* 200 8 */ }; /* 192 16 */ } __attribute__((__aligned__(64))); /* 192 64 */ /* --- cacheline 4 boundary (256 bytes) --- */ uint16_t size; /* 256 2 */ However, if I remove the explicit align, I get what I expect: /* --- cacheline 3 boundary (192 bytes) --- */ union { uint8_t ctx[16]; /* 192 16 */ struct { void * ctx_ptr; /* 192 8 */ void * ctx_ptr2; /* 200 8 */ }; /* 192 16 */ }; /* 192 16 */ uint16_t size; /* 208 2 */ Is it OK to drop the explicit alignment? This is beyond my C skills :) > > +=09=09uint8_t ctx[RTE_NODE_CTX_SZ]; > > +=09=09/* Convenience aliases to store pointers without complex casting= . */ > > +=09=09__extension__ struct { > > this is correct/recommended since anonymous structs aren't standard, > with the __extension__ -pedantic won't emit a warning (our intention). Ack. > > +static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_no= de, ctx) =3D=3D RTE_NODE_CTX_SZ, > > +=09"The node context anonymous union cannot be larger than RTE_NODE_CT= X_SZ"); > > + > > you should include directly include in this file for use of of= fsetof. > you should include directly include in this file for use of th= e static_assert. Will do for v3. Thanks!