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 C1F8D438E9; Wed, 17 Jan 2024 10:57:36 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 42FD0402B0; Wed, 17 Jan 2024 10:57:36 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id EC6F6402A6 for ; Wed, 17 Jan 2024 10:57:34 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id BC545205F3; Wed, 17 Jan 2024 10:57:34 +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 v3 1/5] event/opdl: fix non-constant compile time assertion X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 17 Jan 2024 10:57:30 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F16D@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v3 1/5] event/opdl: fix non-constant compile time assertion Thread-Index: AdpJJ1vsXFkgWwNiRaWp+tFLeYW3fAAAkpNg References: <20231111172153.57461-1-stephen@networkplumber.org> <20240116184307.162882-1-stephen@networkplumber.org> <20240116184307.162882-2-stephen@networkplumber.org> <34c1fbc6-ae4a-4789-9974-fcc4d6a74a60@oktetlabs.ru> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" , "Andrew Rybchenko" Cc: "Stephen Hemminger" , , , "Tyler Retzlaff" , "Liang Ma" , "Peter Mccarthy" , =?iso-8859-1?Q?Se=E1n_Harte?= 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: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Wednesday, 17 January 2024 10.27 >=20 > On Wed, Jan 17, 2024 at 10:58:12AM +0300, Andrew Rybchenko wrote: > > On 1/16/24 21:41, Stephen Hemminger wrote: > > > RTE_BUILD_BUG_ON() was being used with a non-constant value. > > > The inline function rte_is_power_of_2() is not constant since > > > inline expansion happens later in the compile process. > > > Replace it with macro which will be constant. > > > > > > Fixes: 4236ce9bf5bf ("event/opdl: add OPDL ring infrastructure > library") > > > Cc: liang.j.ma@intel.com > > > Signed-off-by: Stephen Hemminger > > > Acked-by: Bruce Richardson > > > Acked-by: Tyler Retzlaff > > > --- > > > drivers/event/opdl/opdl_ring.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/event/opdl/opdl_ring.c > b/drivers/event/opdl/opdl_ring.c > > > index 69392b56bbec..24e0bbe3222d 100644 > > > --- a/drivers/event/opdl/opdl_ring.c > > > +++ b/drivers/event/opdl/opdl_ring.c > > > @@ -31,6 +31,9 @@ > > > #define OPDL_OPA_MASK (0xFF) > > > #define OPDL_OPA_OFFSET (0x38) > > > +/* Equivalent to rte_is_power_of_2() but as macro. */ > > > +#define IS_POWER_OF_2(x) (((x) & ((x) - 1)) =3D=3D 0) > > > > IMHO adding it in specific driver is a wrong direction. I'm afraid = it > > will result in duplication of such macros in code base without clear > > reason why. > > > > May be it is better to add it with a proper name to EAL? > > > +1 > Even if it's a lower-case name, lets make it a macro in EAL so we can > use > it everywhere in asserts. Here's a thought... we could introduce a new code convention for this = purpose. E.g. either: 1. Macros in lower case are guaranteed to access each parameter exactly = once (and might not be usable for static_assert; they somewhat resemble = inline functions), and macros in upper case may access parameters any number of times (and must = be usable for static_assert). Or: 2. Macros that access any parameter zero or multiple times must have = their name postfixed _UNSAFE (or some other word).