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 E4B8B43D47; Mon, 25 Mar 2024 12:00:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C81FC40271; Mon, 25 Mar 2024 12:00:14 +0100 (CET) Received: from mail-qt1-f182.google.com (mail-qt1-f182.google.com [209.85.160.182]) by mails.dpdk.org (Postfix) with ESMTP id 44FC840270 for ; Mon, 25 Mar 2024 12:00:13 +0100 (CET) Received: by mail-qt1-f182.google.com with SMTP id d75a77b69052e-430ddb1a227so27731451cf.3 for ; Mon, 25 Mar 2024 04:00:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711364412; x=1711969212; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=CRPs8Kv/omEpSPlnFET3H/NAzudaj92p0Ju0gSOdRAA=; b=DNCy9z+I12szt5xNTYZ8NnwJg1tRRnPxb38cTOPQj8BIkKtw32AXqlBqs3mEaV7xnS VjNGNaUyp9Bcbt27ypIgWM4kGvKqc7BFymG7HP3OtUCmKF+89dlFBhG4Nix5fGlxDg4v WTx/f9TsnWEckNfsSKWTZenlMLtHtlDJ84xDBmhyyI6NH9Kp9PFdQV2NjU4VxEUDZ4ZB 4io584sypPARTO8ajiU1idXyav5C5VYAxGXpqnSlJnzx+Y9Grq1mRU6/Qf9YtzfE1uB3 pGxv1171gAQCfsy4g3Pv1u5zduanjOdUWnDje4jD0kGmAf1G8Ydaf3B5wri1KaICbuZ9 8Eyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711364412; x=1711969212; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CRPs8Kv/omEpSPlnFET3H/NAzudaj92p0Ju0gSOdRAA=; b=RNZ4ONDs+azgkCTuTQEdNyxGGzu/n6pUSd/1FbRBjEMpF5Ar4u+CKi4krYQP/VbaZG 47PO3E4HSlJSjB1Tga8EO64hlve76YlWIxyim5o/sAtKWm33S9ds0kJoBwt7qhkLU8+4 Nw6w3xUEOnegUhm/JblEnyIzOr0WwG4jqQ00w18aaVNcoK19vZCDD7YxTFkjlft5EE5P HIu/MN/UJGaxJsjiFIzmGvd9z1yVGTpFXYc198omwgIj1KwFHrXhM7isqcj6brYPqpav yOxup76WMtMZUeDni/xpEuxTFQ329XxlhAgGIh5q1vv306ZP/RvR+sd68IbcUb/E+3Vc Nq6w== X-Gm-Message-State: AOJu0YzQ3Kq03UL8owdzJLSBjGe+Yb+OV3elms5GJDHXqlVxrDv6R4lr Y035TQoyxTlPIunpXLrfm4PcECZtotqGbDLL5r4p9OhrQeRpocOR0iyRQhk6dWwfrc9osNSAgje xJBTprmbP5YkjBECKLh1QAXexuvc= X-Google-Smtp-Source: AGHT+IEMIDLZAp+sUQK6V7tNMn0TBjC6WmvK9eJQcoh186HL/HHxX/mQ5L6B6DHTo2xZdENK2r4SG2DDktgUPTZrf4Q= X-Received: by 2002:ac8:5c4f:0:b0:431:56ee:6d47 with SMTP id j15-20020ac85c4f000000b0043156ee6d47mr3196638qtj.54.1711364411135; Mon, 25 Mar 2024 04:00:11 -0700 (PDT) MIME-Version: 1.0 References: <20240325100500.694748-2-rjarry@redhat.com> In-Reply-To: <20240325100500.694748-2-rjarry@redhat.com> From: Jerin Jacob Date: Mon, 25 Mar 2024 16:29:44 +0530 Message-ID: Subject: Re: [PATCH v3] graph: expose node context as pointers To: Robin Jarry Cc: dev@dpdk.org, Jerin Jacob , Kiran Kumar K , Nithin Dabilpuram , Zhirun Yan , Tyler Retzlaff Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Mon, Mar 25, 2024 at 3:35=E2=80=AFPM Robin Jarry wro= te: > > In some cases, the node context data is used to store two pointers > because the data is larger than the reserved 16 bytes. Having to define > intermediate structures just to be able to cast is tedious. Add two > pointers that take the same space than ctx. > > Signed-off-by: Robin Jarry > --- > > Notes: > v3: > > * Added __extension__ to the unnamed struct inside the union. > * Fixed C++ header checks. > * Replaced alignas() with an explicit static_assert. > > v2: > > * Added __extension__ (not sure where it is needed, I don't have acce= ss to windows). > * It still fails the header check for C++. It seems not possible to a= lign an unnamed union... > Tyler, do you have an idea about how to fix that? > * Added static_assert to ensure the anonymous union is not larger tha= n RTE_NODE_CTX_SZ. > > lib/graph/rte_graph_worker_common.h | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_wo= rker_common.h > index 36d864e2c14e..722e9dac0d36 100644 > --- a/lib/graph/rte_graph_worker_common.h > +++ b/lib/graph/rte_graph_worker_common.h > @@ -12,7 +12,9 @@ > * process, enqueue and move streams of objects to the next nodes. > */ > > +#include > #include > +#include > > #include > #include > @@ -112,7 +114,19 @@ struct __rte_cache_aligned rte_node { > }; > /* Fast path area */ > #define RTE_NODE_CTX_SZ 16 > - alignas(RTE_CACHE_LINE_SIZE) uint8_t ctx[RTE_NODE_CTX_SZ]; /**< N= ode Context. */ > + /* > + * alignas(RTE_CACHE_LINE_SIZE) cannot be used for ctx since it i= s part of an unnamed union. > + * The compiler shifts the next field on the next cache line whic= h is not what we want. > + * The alignment is enforced via a explcicit static asserts below= . > + */ > + union { > + uint8_t ctx[RTE_NODE_CTX_SZ]; > + /* Convenience aliases to store pointers without complex = casting. */ > + __extension__ struct { > + void *ctx_ptr; > + void *ctx_ptr2; > + }; > + }; /**< Node Context. */ > uint16_t size; /**< Total number of objects available. *= / > uint16_t idx; /**< Number of objects used. */ > rte_graph_off_t off; /**< Offset of node in the graph reel. */ > @@ -130,6 +144,11 @@ struct __rte_cache_aligned rte_node { > alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node *nodes[]; /**< N= ext nodes. */ > }; > > +static_assert(offsetof(struct rte_node, ctx) % RTE_CACHE_LINE_SIZE =3D= =3D 0, > + "rte_node ctx must be aligned on a cache line"); This will fail in 32bit machine. https://mails.dpdk.org/archives/test-report/2024-March/623806.html I can think of following solution to add before ctx. RTE_MARKER fastpath __rte_cache__aligned; > +static_assert(offsetof(struct rte_node, size) - offsetof(struct rte_node= , ctx) =3D=3D RTE_NODE_CTX_SZ, > + "rte_node context union cannot be larger than RTE_NODE_CTX_SZ"); > + > /** > * @internal > * > -- > 2.44.0 >