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 D734B43093; Thu, 17 Aug 2023 21:14:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5D9BF406A2; Thu, 17 Aug 2023 21:14:11 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 2CFD04013F; Thu, 17 Aug 2023 21:14:09 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 7BB12211F7BA; Thu, 17 Aug 2023 12:14:08 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 7BB12211F7BA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1692299648; bh=ieomRHzvcxooTp2qSFeRdmFxLSYLh5c1EC9/BcafDdQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QOlBFVkQ6vunnKuTluxDBOxgsrlHphSL9pjKJGKsEYB1j0IJVE/i1MszLCnx6ejqj YMYGYumK6ibVcFK4x54X0gOhmFS3fdX2YHo+/bcYA3YIUuyl9bRHYEDhJmJXAcYOfd kWaiK7P5CMhq6vsBDGNCNQE51z+eQPd/Vdx01+Ng= Date: Thu, 17 Aug 2023 12:14:08 -0700 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: dev@dpdk.org, techboard@dpdk.org, Bruce Richardson , Honnappa Nagarahalli , Ruifeng Wang , Jerin Jacob , Sunil Kumar Kori , Mattias =?iso-8859-1?Q?R=F6nnblom?= , Joyce Kong , David Christensen , Konstantin Ananyev , David Hunt , Thomas Monjalon , David Marchand Subject: Re: [PATCH v4 6/6] devtools: forbid new direct use of GCC atomic builtins Message-ID: <20230817191408.GB12307@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1691717521-1025-1-git-send-email-roretzla@linux.microsoft.com> <1692221931-32334-1-git-send-email-roretzla@linux.microsoft.com> <1692221931-32334-7-git-send-email-roretzla@linux.microsoft.com> <98CBD80474FA8B44BF855DF32C47DC35D87B16@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87B16@smartserver.smartshare.dk> 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, Aug 17, 2023 at 01:57:01PM +0200, Morten Brørup wrote: > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > > Sent: Wednesday, 16 August 2023 23.39 > > > > Refrain from using compiler __atomic_xxx builtins DPDK now requires > > the use of rte_atomic__explicit macros when operating on DPDK > > atomic variables. > > There is probably no end to how much can be added to checkpatches. > > You got the important stuff, so below are only further suggestions! > > [...] > > > - # refrain from using compiler __atomic_{add,and,nand,or,sub,xor}_fetch() > > + # refrain from using compiler __atomic_xxx builtins > > awk -v FOLDERS="lib drivers app examples" \ > > - -v EXPRESSIONS="__atomic_(add|and|nand|or|sub|xor)_fetch\\\(" \ > > + -v EXPRESSIONS="__atomic_.*\\\(" \ > > -v RET_ON_FAIL=1 \ > > - -v MESSAGE='Using __atomic_op_fetch, prefer __atomic_fetch_op' \ > > + -v MESSAGE='Using __atomic_xxx builtins' \ > > Alternatively: > -v MESSAGE='Using __atomic_xxx built-ins, prefer rte_atomic_xxx' \ i can adjust the wording as you suggest, no problem > > > -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > > "$1" || res=1 > > > > -- > > 1.8.3.1 > > This could be updated too: > > # refrain from using compiler __atomic_thread_fence() > # It should be avoided on x86 for SMP case. > awk -v FOLDERS="lib drivers app examples" \ > -v EXPRESSIONS="__atomic_thread_fence\\\(" \ > -v RET_ON_FAIL=1 \ > - -v MESSAGE='Using __atomic_thread_fence' \ > + -v MESSAGE='Using __atomic_thread_fence built-in, prefer __rte_atomic_thread_fence' \ > -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ yeah, i left this one separate i think the advice is actually use rte_atomic_thread_fence which may be an inline function that uses __rte_atomic_thread_fence > "$1" || res=1 > > You could also add C11 variants of these tests... > atomic_(load|store|exchange|compare_exchange_(strong|weak)|fetch_(add|sub|and|xor|or|nand)|flag_(test_and_set|clear))[_explicit], and > atomic_thread_fence. > > And a test for using "_Atomic". direct use would cause early compilation in the CI so it would be caught fairly early. i'm not sure i want to get into the business of trying to add redundant (albiet cheaper) earlier checks. though if there is a general call for this from the reviewers i'll add them. > > -Morten