From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <adrien.mazarguil@6wind.com> Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by dpdk.org (Postfix) with ESMTP id 92C9D1B236 for <dev@dpdk.org>; Tue, 31 Oct 2017 14:21:58 +0100 (CET) Received: by mail-wm0-f41.google.com with SMTP id y80so15654875wmd.0 for <dev@dpdk.org>; Tue, 31 Oct 2017 06:21:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=T7bR4zJcG71e6xF1xgrnCeJXDy+k5XyQXH/nLz3lZFs=; b=ZnbWknsCwUAYdzSeJuwp4XSLhereBkADL8kgpc39emGH61VJUBAYZT/yCOqbx2nuR0 QdnLhAFCjPJyJno42b8ktW6QOV4Kvm1eQXdPXE19o8l8qG7dxsR+cUItwykQzUjz5gvs 52f98CsdeilKfzwdCmVtSBB4zekFy/vsZOn1OO0uZ5mTPQ0/L+qCqdAjUhokOJW5Pzw5 Gw/SFchzSDCvkc98usmEU9xw1GDKnnaTZRGJKY97yfIhKibTNtxrqz3gLdM3MM5rpHCy Aobh/GfzotSDDdt/MRuqqmVWQPhQg8SZHls1AErgzCTBR+MxQLJmUPflgLxG/BPVbeDP kaMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=T7bR4zJcG71e6xF1xgrnCeJXDy+k5XyQXH/nLz3lZFs=; b=B9qWv4qJgxL+BwKrYKiKkPZ9ZJaQaZeOHC5Jj4ZD+DuIMahh0sFhK3Wxizaz+8T3ei 6+IA/We2iaCKNJPAEDN8bHqEdCFeDffRBuX3N6k526+Ek0KkjePS0p3TfgBQZJgfD0Gp lpOzFFb5lCIhb/G6UsHIDfKw9Zp8dhRrhvwjj06mnjYRP8nwHl/VWaK0o5oI5F6aDuiY 1VZ0rta9O/X2DAS/ARHL+1XCkclQus5qCBJKVtlEFg/rcwFNP+1qCdGyf0pwQuxx0z1+ /DKgarPRNXlaGFQqSXLaW8ESN9KmB7huf+fHfnyEf3VdkG/3P0t32uhzwnmu7UGppl1R sgBQ== X-Gm-Message-State: AMCzsaUy0jmpSH1fmKg0eL7/5gCSyvrhkBy6IWF9gFTrNGH913UZVy1z HOvvs0tfVCCF62YerehN8EflVw== X-Google-Smtp-Source: ABhQp+S/tFNXgyE0VSpcnDe1dGJ1p0jDBrYiI5tXeUpeCWA2WlYZcCxOpy2+tKnCtxgYTA5HcbIwDg== X-Received: by 10.28.10.195 with SMTP id 186mr1784501wmk.136.1509456118108; Tue, 31 Oct 2017 06:21:58 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id c37sm4541150wra.73.2017.10.31.06.21.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 31 Oct 2017 06:21:57 -0700 (PDT) Date: Tue, 31 Oct 2017 14:21:45 +0100 From: Adrien Mazarguil <adrien.mazarguil@6wind.com> To: Matan Azrad <matan@mellanox.com> Cc: "dev@dpdk.org" <dev@dpdk.org>, Ophir Munk <ophirmu@mellanox.com> Message-ID: <20171031132145.GO26782@6wind.com> References: <1508768520-4810-1-git-send-email-ophirmu@mellanox.com> <1509358049-18854-1-git-send-email-matan@mellanox.com> <1509358049-18854-7-git-send-email-matan@mellanox.com> <20171030142350.GC26782@6wind.com> <HE1PR0502MB3659B337992E2E7D33E587ABD2590@HE1PR0502MB3659.eurprd05.prod.outlook.com> <20171031101722.GL26782@6wind.com> <HE1PR0502MB36596061864215EC0708F472D25E0@HE1PR0502MB3659.eurprd05.prod.outlook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <HE1PR0502MB36596061864215EC0708F472D25E0@HE1PR0502MB3659.eurprd05.prod.outlook.com> Subject: Re: [dpdk-dev] [PATCH v3 6/7] net/mlx4: mitigate Tx path memory barriers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Tue, 31 Oct 2017 13:21:58 -0000 Hi Matan, On Tue, Oct 31, 2017 at 11:35:29AM +0000, Matan Azrad wrote: <snip> > > -----Original Message----- > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > Sent: Tuesday, October 31, 2017 12:17 PM > > To: Matan Azrad <matan@mellanox.com> > > Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com> > > Subject: Re: [PATCH v3 6/7] net/mlx4: mitigate Tx path memory barriers > > > > Hi Matan, > > > > On Mon, Oct 30, 2017 at 07:47:20PM +0000, Matan Azrad wrote: > > > Hi Adrien > > > > > > > -----Original Message----- > > > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > > > Sent: Monday, October 30, 2017 4:24 PM > > > > To: Matan Azrad <matan@mellanox.com> > > > > Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com> > > > > Subject: Re: [PATCH v3 6/7] net/mlx4: mitigate Tx path memory > > > > barriers > > > > > > > > On Mon, Oct 30, 2017 at 10:07:28AM +0000, Matan Azrad wrote: > > > > > Replace most of the memory barriers by compiler barriers since > > > > > they are all targeted to the DRAM; This improves code efficiency > > > > > for systems which force store order between different addresses. > > > > > > > > > > Only the doorbell record store should be protected by memory > > > > > barrier since it is targeted to the PCI memory domain. > > > > > > > > > > Limit pre byte count store compiler barrier for systems with cache > > > > > line size smaller than 64B (TXBB size). > > > > > > > > > > Signed-off-by: Matan Azrad <matan@mellanox.com> <snip> > > > > > cq->cons_index = cons_index; > > > > > *cq->set_ci_db = rte_cpu_to_be_32(cq->cons_index & > > > > MLX4_CQ_DB_CI_MASK); > > > > > - rte_wmb(); > > > > > + rte_io_wmb(); > > > > > > > > This one could be removed entirely as well, which is more or less > > > > what the move to a compiler barrier does. Nothing in subsequent code > > > > depends on this doorbell being written, so this can piggy back on > > > > any subsequent rte_wmb(). > > > > > > Yes, you right, probably this code was taken from multi thread > > implementation. > > > > > > > > On the other hand in my opinion a barrier (compiler or otherwise) > > > > might be needed before the doorbell write, to make clear it cannot > > > > somehow be done earlier in case something attempts to optimize it > > away. > > > > > > > I think we can remove it entirely (compiler can't optimize the ci_db store > > since in depends in previous code (cons_index). > > > > Right, however you may still run into issues if the compiler determines the > > final cons_index value by looking at the loop and decides to store it before > > entering/leaving it. That's the kind of problematic optimization I was thinking > > of. > > > > The barrier in that sense is just to assert the order of seemingly unrelated > > load/stores. > > I think that If I left the rte_io_rmb after CQE owner check we can earn both: > 1. The concern of read ordering while reading the owner before using CQE. > 2. The ci_db concern: the compiler must read the last CQE(which is not valid and we have no more stamp to do) before it knows the last value of cons_index. > So we can remove entirely this rte_io_wmb in completion function. > What do you think? That's right, this means there's a barrier before the doorbell write in any case, OK then. Just make sure cq->set_ci_db is volatile in a prior "fix" commit as described in my previous suggestion, otherwise the remaining barriers won't guarantee much. Thanks. -- Adrien Mazarguil 6WIND