From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C3565A0546; Thu, 16 Jul 2020 12:49:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1AA8A1BEAC; Thu, 16 Jul 2020 12:49:15 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by dpdk.org (Postfix) with ESMTP id 86C8D1BE9C for ; Thu, 16 Jul 2020 12:49:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594896553; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3AyaIPBjHdgho26xCAeg7EdV+YWndsAGlFTytp4oTNE=; b=hz00l8Fj7MOBdTL3/R8rNapi/gujttzqJ7uYn79A/R4daSeks2DzfG2MIev6dvGMzRRc5L 3BCwWMo5GGtD7hwKwowt6eMDK8kMzkT6dOqTyyKuA/kq4Wo/2e308i+OV7y34tdaq2DXX4 F9wVqIOsTF9DxMq9lq07TUV5kP2QKnI= Received: from mail-vk1-f198.google.com (mail-vk1-f198.google.com [209.85.221.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-248-h0TI0mDhPsSwIPf9IeaIzw-1; Thu, 16 Jul 2020 06:49:11 -0400 X-MC-Unique: h0TI0mDhPsSwIPf9IeaIzw-1 Received: by mail-vk1-f198.google.com with SMTP id n133so1908684vkc.14 for ; Thu, 16 Jul 2020 03:49:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3AyaIPBjHdgho26xCAeg7EdV+YWndsAGlFTytp4oTNE=; b=Y03diWWJ91xZsczX8PRFxxD9L4s6/VVIaQCkl2jm76kjZoF8qdkKoC5ieC0ZdszOUz upoO41YPjB9MejF8sMkSNe0oVcnbNDvh1eGTVAkFLBMSjs6medzJtWXtjvSP40VlB5ZX xBr+2dnuNKC7MDOJDXN35Kc1IksTVKgzeAwT+UFCQaxE0dOgjEmBOmTFhL9aXCKSKwle jcjvpDuZbnw9akjXPyU9J5say4CLLO4ZCouQmcYZiY/cqvbNsJ126n1phKYrESKrTmvV Fiz6H3UtVXaHMB3G8hghJu6OVfbQsaI9S7ZgQ1I2DlqbLDoeciXLBMatXB7I9mLpgZnG 9JSQ== X-Gm-Message-State: AOAM5311Uac1x8hR5PQUmbCUlpqVWB9ed7Uo8+zRKZkE1pwp8mQl/Mkz VYSL+gBh5aCk6GdTnIpabGyVHdFnLw4Q68vaReQWmCaG3qV1K2XbZhPe8cP5RKHgO3WjfFQ56Kd OcajO7xtIC7koxQfDOls= X-Received: by 2002:ab0:316:: with SMTP id 22mr2775590uat.41.1594896551006; Thu, 16 Jul 2020 03:49:11 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw+VdoQZRLiyjoKVqoE24rDyD+gmkStZdkB5fzXXWCyXdBnnCMcxHC8WyyaFEYMmDie3VnGQ8Ug6QpQGrV/fyg= X-Received: by 2002:ab0:316:: with SMTP id 22mr2775569uat.41.1594896550692; Thu, 16 Jul 2020 03:49:10 -0700 (PDT) MIME-Version: 1.0 References: <1594621423-14796-1-git-send-email-phil.yang@arm.com> <1594875225-5850-1-git-send-email-phil.yang@arm.com> <1594875225-5850-3-git-send-email-phil.yang@arm.com> In-Reply-To: <1594875225-5850-3-git-send-email-phil.yang@arm.com> From: David Marchand Date: Thu, 16 Jul 2020 12:48:59 +0200 Message-ID: To: Phil Yang Cc: Thomas Monjalon , "Mcnamara, John" , Honnappa Nagarahalli , David Christensen , dev , Jerin Jacob Kollanukkaran , "Ananyev, Konstantin" , Ola Liljedahl , Bruce Richardson , "Ruifeng Wang (Arm Technology China)" , nd X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v8 2/3] devtools: prevent use of rte atomic APIs in future patches X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jul 16, 2020 at 6:58 AM Phil Yang wrote: > > In order to deprecate the rte_atomic and rte_smp barrier APIs, prevent > the patches from using these APIs in the converted modules and compilers > __sync built-ins in all modules. builtins* for the whole patch. > > The converted modules: > lib/librte_distributor > lib/librte_hash > lib/librte_kni > lib/librte_lpm > lib/librte_rcu > lib/librte_ring > lib/librte_stack > lib/librte_vhost > lib/librte_timer > lib/librte_ipsec > drivers/event/octeontx > drivers/event/octeontx2 > drivers/event/opdl > drivers/net/bnx2x > drivers/net/hinic > drivers/net/hns3 > drivers/net/memif > drivers/net/thunderx > drivers/net/virtio > examples/l2fwd-event > > On x86 the __atomic_thread_fence(__ATOMIC_SEQ_CST) is quite expensive > for SMP case. Flag the new code which use __atomic_thread_fence API. > > Signed-off-by: Phil Yang > Reviewed-by: Ruifeng Wang > --- > devtools/checkpatches.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh > index 58021aa..fb288cf 100755 > --- a/devtools/checkpatches.sh > +++ b/devtools/checkpatches.sh > @@ -51,6 +51,13 @@ print_usage () { > > check_forbidden_additions() { # > res=0 > + c11_atomics_dir="lib/librte_distributor lib/librte_hash lib/librte_kni > + lib/librte_lpm lib/librte_rcu lib/librte_ring > + lib/librte_stack lib/librte_vhost > + drivers/event/octeontx drivers/event/octeontx2 > + drivers/event/opdl drivers/net/bnx2x drivers/net/hinic > + drivers/net/hns3 drivers/net/memif drivers/net/thunderx > + drivers/net/virtio examples/l2fwd-event" I prefer a form like: + c11_atomics_dir="" + c11_atomics_dir="$c11_atomics_dir drivers/event/octeontx" + c11_atomics_dir="$c11_atomics_dir drivers/event/octeontx2" + c11_atomics_dir="$c11_atomics_dir drivers/event/opdl" + c11_atomics_dir="$c11_atomics_dir drivers/net/bnx2x" + c11_atomics_dir="$c11_atomics_dir drivers/net/hinic" + c11_atomics_dir="$c11_atomics_dir drivers/net/hns3" + c11_atomics_dir="$c11_atomics_dir drivers/net/memif" + c11_atomics_dir="$c11_atomics_dir drivers/net/thunderx" + c11_atomics_dir="$c11_atomics_dir drivers/net/virtio" + c11_atomics_dir="$c11_atomics_dir examples/l2fwd-event" + c11_atomics_dir="$c11_atomics_dir lib/librte_distributor" + c11_atomics_dir="$c11_atomics_dir lib/librte_hash" + c11_atomics_dir="$c11_atomics_dir lib/librte_kni" + c11_atomics_dir="$c11_atomics_dir lib/librte_lpm" + c11_atomics_dir="$c11_atomics_dir lib/librte_rcu" + c11_atomics_dir="$c11_atomics_dir lib/librte_ring" + c11_atomics_dir="$c11_atomics_dir lib/librte_stack" + c11_atomics_dir="$c11_atomics_dir lib/librte_vhost" Easier to read and update. > > # refrain from new additions of rte_panic() and rte_exit() > # multiple folders and expressions are separated by spaces > @@ -74,6 +81,39 @@ check_forbidden_additions() { # > -v EXPRESSIONS='for[[:space:]]*\\((char|u?int|unsigned|s?size_t)' \ > -v RET_ON_FAIL=1 \ > -v MESSAGE='Declaring a variable inside for()' \ This hunk breaks the script. This is not seen by the CI, as the CI uses its local copy of the script. We are missing a: + "$1" || res=1 > + > + # refrain from new additions of 16/32/64 bits rte_atomic_xxx() > + # multiple folders and expressions are separated by spaces > + awk -v FOLDERS="$c11_atomics_dir" \ > + -v EXPRESSIONS="rte_atomic[0-9][0-9]_.*\\\(" \ > + -v RET_ON_FAIL=1 \ > + -v MESSAGE='Use of rte_atomicNN_xxx APIs not allowed, use __atomic_xxx built-ins' \ Other messages in this script report what is wrong. Example: -v MESSAGE='Using rte_panic/rte_exit' \ -v MESSAGE='Using compiler attribute directly' \ -v MESSAGE='Declaring a variable inside for()' \ So to be consistent, + -v MESSAGE='Using rte_atomicNN_xxx' \ > + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > + "$1" || res=1 > + > + # refrain from new additions of rte_smp_XXmb() > + # multiple folders and expressions are separated by spaces > + awk -v FOLDERS="$c11_atomics_dir" \ > + -v EXPRESSIONS="rte_smp_(r|w)?mb\\\(" \ > + -v RET_ON_FAIL=1 \ > + -v MESSAGE='Use of rte_smp_[r/w]mb not allowed, use __atomic_xxx built-ins' \ + -v MESSAGE='Using rte_smp_[r/w]mb' \ > + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > + "$1" || res=1 > + > + # refrain from using compiler __sync built-ins > + awk -v FOLDERS="lib drivers app examples" \ > + -v EXPRESSIONS="__sync_.*\\\(" \ > + -v RET_ON_FAIL=1 \ > + -v MESSAGE='Use of __sync_xxx built-ins not allowed, use __atomic_xxx built-ins' \ + -v MESSAGE='Using __sync_xxx builtins' \ > + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > + "$1" || res=1 > + > + # 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='The __atomic_thread_fence is not allowed, use rte_atomic_thread_fence' \ + -v MESSAGE='Using __atomic_thread_fence' \ > -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > "$1" || res=1 > -- David Marchand