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 57D77436FA; Fri, 15 Dec 2023 08:17:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2E6C24067E; Fri, 15 Dec 2023 08:17:03 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 00E4440649 for ; Fri, 15 Dec 2023 08:17:00 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 3811A20B3CC1; Thu, 14 Dec 2023 23:17:00 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3811A20B3CC1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1702624620; bh=oWLx6v79MFH9PR7ZhE5ldA1iqsKe762CJrgE33/Q/Bo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=fgCalkvBNfUHCTlMeuKshtWJlDG9VuVoNyr4tOGivVBs+1Zlj0JBfncpRaPTjvLa8 Qds0TPthK8vkj3blPohqgAsTSIZhuj7W3awVIPbY1bZM04aGtNuQo/lJPzDvWjJKwJ fKp/EZb5aM+Z5p5befw/DAS2STGYSMJcmdmkh1Ic= Date: Thu, 14 Dec 2023 23:17:00 -0800 From: Tyler Retzlaff To: Jie Hai Cc: dev@dpdk.org, Morten =?iso-8859-1?Q?Br=F8rup?= , Konstantin Ananyev , lihuisong@huawei.com, fengchengwen@huawei.com, liudongdong3@huawei.com Subject: Re: [PATCH 1/2] eal: fix constraints on stdatomic API Message-ID: <20231215071700.GA17288@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20231211073904.811243-1-haijie1@huawei.com> <20231211073904.811243-2-haijie1@huawei.com> <20231211185327.GA16826@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <041426af-9b1c-e827-749c-35930dd7f6e2@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <041426af-9b1c-e827-749c-35930dd7f6e2@huawei.com> 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 Fri, Dec 15, 2023 at 10:47:36AM +0800, Jie Hai wrote: > On 2023/12/12 2:53, Tyler Retzlaff wrote: > >On Mon, Dec 11, 2023 at 03:39:03PM +0800, Jie Hai wrote: > >>The first parameter of rte_atomic_exchange_explicit() must be a > >>pointer to _Atomic type. If run command "meson setup --werror > >>-Denable_stdatomic=true build && ninja -C build", error will occur. > >>Thia patch fixes it. > >> > >>Fixes: 1ec6a845b5cb ("eal: use stdatomic API in public headers") > >>Cc: stable@dpdk.org > >> > >>Signed-off-by: Jie Hai > >>--- > >> app/test/test_atomic.c | 6 +++--- > >> lib/eal/include/generic/rte_atomic.h | 12 ++++++------ > >> 2 files changed, 9 insertions(+), 9 deletions(-) > >> > >>diff --git a/app/test/test_atomic.c b/app/test/test_atomic.c > >>index db07159e81ab..c3cb3ae0ea57 100644 > >>--- a/app/test/test_atomic.c > >>+++ b/app/test/test_atomic.c > >>@@ -347,9 +347,9 @@ typedef union { > >> const uint8_t CRC8_POLY = 0x91; > >> uint8_t crc8_table[256]; > >>-volatile uint16_t token16; > >>-volatile uint32_t token32; > >>-volatile uint64_t token64; > >>+volatile RTE_ATOMIC(uint16_t) token16; > >>+volatile RTE_ATOMIC(uint32_t) token32; > >>+volatile RTE_ATOMIC(uint64_t) token64; > > > >subject to my comment below, volatile qualification can be removed. > > > >> static void > >> build_crc8_table(void) > >>diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h > >>index 0e639dad76a4..38c3b41f9c68 100644 > >>--- a/lib/eal/include/generic/rte_atomic.h > >>+++ b/lib/eal/include/generic/rte_atomic.h > >>@@ -207,11 +207,11 @@ rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src) > >> * The original value at that location > >> */ > >> static inline uint16_t > >>-rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val); > >>+rte_atomic16_exchange(volatile RTE_ATOMIC(uint16_t) *dst, uint16_t val); > > > >the existing rte_atomicNN (the old non-standard ones) are deprecated and will > >be eventually removed so there isn't a lot of value in churning their > >signatures to wrap the rte_stdatomic macros. > > > >the right thing to do here to just change the calling code to use the generic > >rte_stdatomic macros directly so we can eventually remove > >rte_atomicNN_xxx. > > > >ty > > > Hi, Tyler Retzlaff, > > Thank you for your review. > > As I understand it, this code is used to test the API > rte_atomXXX_change, and the call here should not be modified. > > Since the current problem affects compilation, I think it can be > solved first. okay, i understand the motivation now and see what you mean. first, sorry for the trouble i did not expect anyone to start using this option until i had completed full conversion of the tree. drivers and tests are still on my todo list. for now would it be reasonable to just stop building this test when enable_stdatomic=true? the api are still going to be tested by the ci and builds that do not enable the option. as for changing the signatures of the existing api i don't strictly object since the RTE_ATOMIC() macro expands empty for the non stdatomic builds so isn't technically an api or abi change. but there may still be some resistance to merging regardless. wonder if anyone else has any suggestions here? ty > > What do you think? > > Thanks, > Jie Hai > >.