From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com [209.85.212.179]) by dpdk.org (Postfix) with ESMTP id CACCD678B for ; Wed, 16 Apr 2014 10:55:10 +0200 (CEST) Received: by mail-wi0-f179.google.com with SMTP id z2so1015271wiv.6 for ; Wed, 16 Apr 2014 01:55:11 -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:organization:user-agent :mime-version:to:cc:subject:references:in-reply-to:content-type; bh=GIABvpmstrNBd4NG40PPy8s5L/zj3jnrBgMTYyLKdKs=; b=Vvur0ZdQFtG/odp1ttt+FRKapKkaXsif/YTnuNgcprgEkVW+bZrQK5p7YlhVWXu/9l 0cE6SfAio5nFqcPPJtfRDIFUealSmotH6Zv1AygaLqxZjI5H6p/4KlYavPVU853Rx4HN I8ScyXyqndEdc1s+w5Ao7VNDW/y8UtuSKmrbaAi5Mc2ST3b/NxHO9Cd7Uc5Z9WgtGg5M 7g2lun3dJ3XsOJ5rLRGUEzb1HAplsektbxIwi8w5h29Ap4W87WIVvTfBXf4qlvxADR80 fQGBtei5Y1fNedxefiCJnYBOWrmu8H+v9fH56Qfb30ISA1O8CtcoyIZs01RU7rrJylfE AHXA== X-Gm-Message-State: ALoCoQkOgDLEF0qAqyJQoexgoY20CQhNGj9sIMTHdUjnwzcs6a6owO9OpDSP+oyhe5tduL1hpT39 X-Received: by 10.180.99.161 with SMTP id er1mr18504655wib.3.1397638511163; Wed, 16 Apr 2014 01:55:11 -0700 (PDT) Received: from [10.16.0.123] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id kp5sm33390172wjb.30.2014.04.16.01.55.09 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 16 Apr 2014 01:55:10 -0700 (PDT) Message-ID: <534E456B.8080909@6wind.com> Date: Wed, 16 Apr 2014 10:55:07 +0200 From: "didier.pallard" Organization: 6WIND User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:18.0) Gecko/20121128 Thunderbird/18.0 MIME-Version: 1.0 To: "Richardson, Bruce" References: <1397569859-14460-1-git-send-email-david.marchand@6wind.com> <59AF69C657FD0841A61C55336867B5B01A9FCCFF@IRSMSX103.ger.corp.intel.com> In-Reply-To: <59AF69C657FD0841A61C55336867B5B01A9FCCFF@IRSMSX103.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 1/2] mem: add write memory barrier before changing heap state 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: Wed, 16 Apr 2014 08:55:11 -0000 On 04/15/2014 04:08 PM, Richardson, Bruce wrote: >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand >> Sent: Tuesday, April 15, 2014 2:51 PM >> To: dev@dpdk.org >> Subject: [dpdk-dev] [PATCH 1/2] mem: add write memory barrier before >> changing heap state >> >> From: Didier Pallard >> >> a write memory barrier is needed before changing heap state value, else some >> concurrent core may see state changing before all initialization values are >> written to memory, causing unpredictable results in malloc function. >> >> Signed-off-by: Didier Pallard > No barrier should be necessary here. As in a number of other places, such as rings, compiler barriers can be used in place of write memory barriers, due to IA ordering rules. However, in this case, both variables referenced are volatile variables and so the assignments to them cannot be reordered by the compiler so no compiler barrier is necessary either. > > Regards, > /Bruce Hi bruce, Indeed a compiler barrier is absolutely needed here. volatile variable use is absolutely not a serializing instruction from compiler point of view; only atomic variable use is serializing, due to asm volatile (memory) directive use. Here is the assembler generated with and without rte_wmb: With rte_wmb 142: f0 45 0f b1 07 lock cmpxchg %r8d,(%r15) 147: 0f 94 c0 sete %al 14a: 84 c0 test %al,%al 14c: 74 ea je 138 14e: 49 c7 47 10 00 00 00 movq $0x0,0x10(%r15) 155: 00 156: 41 c7 47 18 00 00 00 movl $0x0,0x18(%r15) 15d: 00 15e: 41 c7 47 08 00 00 00 movl $0x0,0x8(%r15) 165: 00 166: 41 c7 47 1c 00 00 00 movl $0x0,0x1c(%r15) 16d: 00 16e: 49 c7 47 20 00 00 00 movq $0x0,0x20(%r15) 175: 00 176: 45 89 57 04 mov %r10d,0x4(%r15) 17a: 0f ae f8 sfence * 17d: 41 c7 07 02 00 00 00 movl $0x2,(%r15)** ** 184: 41 8b 37 mov (%r15),%esi** * 187: 83 fe 02 cmp $0x2,%esi 18a: 75 b4 jne 140 18c: 0f 1f 40 00 nopl 0x0(%rax) 190: 48 83 c3 3f add $0x3f,%rbx Without rte_wmb 142: f0 45 0f b1 07 lock cmpxchg %r8d,(%r15) 147: 0f 94 c0 sete %al 14a: 84 c0 test %al,%al 14c: 74 ea je 138 14e: 49 c7 47 10 00 00 00 movq $0x0,0x10(%r15) 155: 00 156: 41 c7 47 08 00 00 00 movl $0x0,0x8(%r15) 15d: 00 * 15e: 41 c7 07 02 00 00 00 movl $0x2,(%r15)** ** 165: 41 8b 37 mov (%r15),%esi** * 168: 41 c7 47 18 00 00 00 movl $0x0,0x18(%r15) 16f: 00 170: 41 c7 47 1c 00 00 00 movl $0x0,0x1c(%r15) 177: 00 178: 49 c7 47 20 00 00 00 movq $0x0,0x20(%r15) 17f: 00 180: 45 89 57 04 mov %r10d,0x4(%r15) 184: 83 fe 02 cmp $0x2,%esi 187: 75 b7 jne 140 189: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 190: 48 83 c3 3f add $0x3f,%rbx It's clear that the *heap->initialised = INITIALISED;* instruction has been reordered by the compiler. About rte_wmb and rte_rmb use, i agree with you that on intel architecture those macro should do nothing more than compiler barrier, due to Intel architecture choices. But for code completness, i think those memory barriers should remain in place in the code, and rte_*mb should map to compiler barrier on intel architecture. didier