From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f48.google.com (mail-wg0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id 607F1312 for ; Tue, 20 May 2014 14:12:30 +0200 (CEST) Received: by mail-wg0-f48.google.com with SMTP id b13so394085wgh.31 for ; Tue, 20 May 2014 05:12:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=k6jC9zltjW6H2+YobYvTaPD/6Zc+CWvLCyyEuHkUPWk=; b=JhzRNfk4IlljMqS1enhQl5j5fpnI2QN17L7xWQyHoK+eAn+nFlLZ7djRUKjwLi5Bw5 uBAYeHTGuuzMwnFqTUnBCHnfUlLl9R0zO3ttAaR4yJX2zuyfPyVuLfWbr3XM+FGhMUIl Tjjp2qkg7OUccDqmXZ3veHRVhZxvnM7AOIKK52cDcl0An0Ou+xqfQ2JD9tLhzDrhGxbY RajoNOLSc8/f6skwKTM2tsOlNdi6iZANrKR1AlSm754Swx9XxL1+V9f5zcXtIAuOKLEQ jAie6DJe/zy1FBNJ95IbAOKUE5CR/1q0BJF+aMkRlNJZ81NKKThuTQvP1PU6HkUOciMp d08g== X-Gm-Message-State: ALoCoQkUfgAJa/wHeowbR16DzyvRePY6VnEAm0SZPW5UuV1uZuQiZHEICmjB2Bx54LnR1B+baZcg X-Received: by 10.194.242.136 with SMTP id wq8mr15494568wjc.4.1400587959412; Tue, 20 May 2014 05:12:39 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id z14sm20469894wiu.6.2014.05.20.05.12.37 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 20 May 2014 05:12:38 -0700 (PDT) Message-ID: <537B46B4.4000202@6wind.com> Date: Tue, 20 May 2014 14:12:36 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.4.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" , "dev@dpdk.org" References: <1400578588-21137-1-git-send-email-olivier.matz@6wind.com> <2601191342CEEE43887BDE71AB9772580EFA776F@IRSMSX105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772580EFA776F@IRSMSX105.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] atomic: clarify use of memory barriers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 May 2014 12:12:30 -0000 Hi Konstantin, Thank you for your review and feedback. On 05/20/2014 12:05 PM, Ananyev, Konstantin wrote: >> Note that on x86 CPUs, memory barriers between different cores can be guaranteed by a simple compiler barrier. > > I don't think this is totally correct. > Yes, for Intel cpus in many cases memory barrier could be avoided due to nearly strict memory ordering. > Though there are few cases where reordering is possible and when fence instructions would be needed. I tried to mimic the behavior of linux that differentiates *mb() from smp_*mb(), but I did too fast. In linux, we have [1]: smp_mb() = mb() = asm volatile("mfence":::"memory") smp_rmb() = compiler_barrier() smp_wmb() = compiler_barrier() At least this should fixed in the patch. By the way, just for reference, the idea of the patch came from a discussion we had on the list [2]. > For me: > +#define rte_smp_rmb() rte_compiler_barrier() > Seems a bit misleading, as there is no real fence. > So I suggest we keep rte_compiler_barrier() naming and usage. The objectives of the patch (which was probably not explained very clearly in the commit log) were: - make the code more readable to distinguish between the 2 kinds of memory barrier. - optimize some code to avoid a real memory barrier when not required (timers, virtio, ...) Having a compiler barrier in place of a memory barrier in the code does not really help to understand what the developper wanted to do. In the current code we can see that the use of rte_compiler_barrier() is ambiguous, as it need a comment to clarify the situation: rte_compiler_barrier(); /* rmb */ Don't you think we could fix the patch but keep its logic? Regards, Olivier [1] http://lxr.free-electrons.com/source/arch/x86/include/asm/barrier.h#L81 [2] http://dpdk.org/ml/archives/dev/2014-March/001741.html