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 8A92F43FE4; Thu, 9 May 2024 18:47:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 101D3402D7; Thu, 9 May 2024 18:47:15 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 002BA402C2 for ; Thu, 9 May 2024 18:47:12 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 1DA3C20B2C87; Thu, 9 May 2024 09:47:12 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1DA3C20B2C87 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1715273232; bh=1pqHEYj4UzbZfcEuNj342p3DgtZdgaNpKkEN8Q/D5tU=; h=Date:From:To:Subject:References:In-Reply-To:From; b=NacxtYNlTiWM0swZHIqQEAmUWNeu6e29qoa9m5D8rCgItVC8dhvgcdpRWwPvIjHRW NiYopsE5u/9LchBQw00z+y1W5Nie2VzSZNP+1FkA5z2gqF5YGHaVR+xvIqmTzg0mNf i+6p570uWz9px6cvftl5NJ++Fg2Z12ramtkX93Mg= Date: Thu, 9 May 2024 09:47:12 -0700 From: Tyler Retzlaff To: Stephen Hemminger , Ruifeng Wang , dev@dpdk.org, Punit Agrawal , Liang Ma Subject: Re: [PATCH] eal/arm: replace RTE_BUILD_BUG on non-constant Message-ID: <20240509164712.GA2349@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20240502142116.63760-1-daniel.gregory@bytedance.com> <20240503180236.3dd0ee2c@hermes.local> <20240509111150.GA2730096@ste-uk-lab-gw> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240509111150.GA2730096@ste-uk-lab-gw> 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 Thu, May 09, 2024 at 12:11:50PM +0100, Daniel Gregory wrote: > On Fri, May 03, 2024 at 06:02:36PM -0700, Stephen Hemminger wrote: > > On Thu, 2 May 2024 15:21:16 +0100 > > Daniel Gregory wrote: > > > > > The ARM implementation of rte_pause uses RTE_BUILD_BUG_ON to check > > > memorder, which is not constant. This causes compile errors when it is > > > enabled with RTE_ARM_USE_WFE. eg. > > > > > > ../lib/eal/arm/include/rte_pause_64.h: In function ‘rte_wait_until_equal_16’: > > > ../lib/eal/include/rte_common.h:530:56: error: expression in static assertion is not constant > > > 530 | #define RTE_BUILD_BUG_ON(condition) do { static_assert(!(condition), #condition); } while (0) > > > | ^~~~~~~~~~~~ > > > ../lib/eal/arm/include/rte_pause_64.h:156:9: note: in expansion of macro ‘RTE_BUILD_BUG_ON’ > > > 156 | RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire && > > > | ^~~~~~~~~~~~~~~~ > > > > > > This has been the case since the switch to C11 assert (537caad2). Fix > > > the compile errors by replacing the check with an RTE_ASSERT. > > > > > > Signed-off-by: Daniel Gregory > > > > The only calls to rte_wait_until_equal_16 in upstream code > > are in the test_bbdev_perf.c and test_timer.c. Looks like > > these test never got fixed to use rte_memory_order instead of __ATOMIC_ defines. > > Apologies, the commit message could make it clearer, but this is also an > issue for rte_wait_until_equal_32 and rte_wait_until_equal_64. > > rte_wait_until_equal_32 is used in a dozen or so lock tests with the old > __ATOMIC_ defines, as well as rte_ring_generic_pvt.h and > rte_ring_c11_pvt.h, where it's used with the new rte_memorder_order > values. Correct me if I'm wrong, but shouldn't the static assertions in > rte_stdatomic.h ensure that mixed usage doesn't cause any issues, even > if using the older __ATOMIC_ defines isn't ideal? this is just informational. the static assertions are intended to make sure there is alignment between the value produced by the rte_memory_order and __ATOMIC_ constant expressions. so you can expect that intermixing __ATOMIC_ and rte_memory_order should work. the older __ATOMIC_ are still used in tests because i just haven't had time to finish conversion. i have a series up now that makes most of the conversions, it is waiting for review. > > > And there should be a CI test for ARM that enables the WFE code at least > > to ensure it works! > > Yes, that could've caught this sooner.