From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754195Ab3J3NaW (ORCPT ); Wed, 30 Oct 2013 09:30:22 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:51044 "EHLO e06smtp10.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751607Ab3J3NaV (ORCPT ); Wed, 30 Oct 2013 09:30:21 -0400 In-Reply-To: <20131030092725.GL4126@linux.vnet.ibm.com> References: <12083.1382486094@ale.ozlabs.ibm.com> <20131023141948.GB3566@localhost.localdomain> <20131025173749.GG19466@laptop.lan> <20131028132634.GO19466@laptop.lan> <20131028163418.GD4126@linux.vnet.ibm.com> <20131028201735.GA15629@redhat.com> <20131030092725.GL4126@linux.vnet.ibm.com> Subject: Re: perf events ring buffer memory barrier on powerpc X-KeepSent: C6B7100E:31C6B190-42257C14:004841C6; type=4; name=$KeepSent To: paulmck@linux.vnet.ibm.com Cc: Anton Blanchard , Benjamin Herrenschmidt , Frederic Weisbecker , LKML , Linux PPC dev , Mathieu Desnoyers , Michael Ellerman , Michael Neuling , Oleg Nesterov , Peter Zijlstra X-Mailer: Lotus Notes Release 8.5.3 September 15, 2011 Message-ID: From: Victor Kaplansky Date: Wed, 30 Oct 2013 15:28:54 +0200 X-MIMETrack: Serialize by Router on D06ML319/06/M/IBM(Release 8.5.3FP5|July 31, 2013) at 30/10/2013 15:28:47 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13103013-4966-0000-0000-00000757E163 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org "Paul E. McKenney" wrote on 10/30/2013 11:27:25 AM: > If you were to back up that insistence with a description of the orderings > you are relying on, why other orderings are not important, and how the > important orderings are enforced, I might be tempted to pay attention > to your opinion. > > Thanx, Paul NP, though, I feel too embarrassed to explain things about memory barriers when one of the authors of Documentation/memory-barriers.txt is on cc: list ;-) Disclaimer: it is anyway impossible to prove lack of *any* problem. Having said that, lets look into an example in Documentation/circular-buffers.txt: > THE PRODUCER > ------------ > > The producer will look something like this: > > spin_lock(&producer_lock); > > unsigned long head = buffer->head; > unsigned long tail = ACCESS_ONCE(buffer->tail); > > if (CIRC_SPACE(head, tail, buffer->size) >= 1) { > /* insert one item into the buffer */ > struct item *item = buffer[head]; > > produce_item(item); > > smp_wmb(); /* commit the item before incrementing the head */ > > buffer->head = (head + 1) & (buffer->size - 1); > > /* wake_up() will make sure that the head is committed before > * waking anyone up */ > wake_up(consumer); > } > > spin_unlock(&producer_lock); We can see that authors of the document didn't put any memory barrier after "buffer->tail" read and before "produce_item(item)" and I think they have a good reason. Lets consider an imaginary smp_mb() right before "produce_item(item);". Such a barrier will ensure that - - the memory read on "buffer->tail" is completed before store to memory pointed by "item" is committed. However, the store to "buffer->tail" anyway cannot be completed before conditional branch implied by "if ()" is proven to execute body statement of the if(). And the latter cannot be proven before read of "buffer->tail" is completed. Lets see this other way. Lets imagine that somehow a store to the data pointed by "item" is completed before we read "buffer->tail". That would mean, that the store was completed speculatively. But speculative execution of conditional stores is prohibited by C/C++ standard, otherwise any conditional store at any random place of code could pollute shared memory. On the other hand, if compiler or processor can prove that condition in above if() is going to be true (or if speculative store writes the same value as it was before write), the speculative store *is* allowed. In this case we should not be bothered by the fact that memory pointed by "item" is written before a read from "buffer->tail" is completed. Regards, -- Victor From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e06smtp10.uk.ibm.com (e06smtp10.uk.ibm.com [195.75.94.106]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "e06smtp10.uk.ibm.com", Issuer "GeoTrust SSL CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 301642C037E for ; Thu, 31 Oct 2013 00:29:07 +1100 (EST) Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 30 Oct 2013 13:29:04 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 662C517D8057 for ; Wed, 30 Oct 2013 13:28:35 +0000 (GMT) Received: from d06av11.portsmouth.uk.ibm.com (d06av11.portsmouth.uk.ibm.com [9.149.37.252]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9UDSmR060948644 for ; Wed, 30 Oct 2013 13:28:48 GMT Received: from d06av11.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av11.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9UDT01K017103 for ; Wed, 30 Oct 2013 07:29:01 -0600 In-Reply-To: <20131030092725.GL4126@linux.vnet.ibm.com> References: <12083.1382486094@ale.ozlabs.ibm.com> <20131023141948.GB3566@localhost.localdomain> <20131025173749.GG19466@laptop.lan> <20131028132634.GO19466@laptop.lan> <20131028163418.GD4126@linux.vnet.ibm.com> <20131028201735.GA15629@redhat.com> <20131030092725.GL4126@linux.vnet.ibm.com> Subject: Re: perf events ring buffer memory barrier on powerpc To: paulmck@linux.vnet.ibm.com Message-ID: From: Victor Kaplansky Date: Wed, 30 Oct 2013 15:28:54 +0200 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII Cc: Michael Neuling , Mathieu Desnoyers , Peter Zijlstra , LKML , Oleg Nesterov , Linux PPC dev , Anton Blanchard , Frederic Weisbecker List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , "Paul E. McKenney" wrote on 10/30/2013 11:27:25 AM: > If you were to back up that insistence with a description of the orderings > you are relying on, why other orderings are not important, and how the > important orderings are enforced, I might be tempted to pay attention > to your opinion. > > Thanx, Paul NP, though, I feel too embarrassed to explain things about memory barriers when one of the authors of Documentation/memory-barriers.txt is on cc: list ;-) Disclaimer: it is anyway impossible to prove lack of *any* problem. Having said that, lets look into an example in Documentation/circular-buffers.txt: > THE PRODUCER > ------------ > > The producer will look something like this: > > spin_lock(&producer_lock); > > unsigned long head = buffer->head; > unsigned long tail = ACCESS_ONCE(buffer->tail); > > if (CIRC_SPACE(head, tail, buffer->size) >= 1) { > /* insert one item into the buffer */ > struct item *item = buffer[head]; > > produce_item(item); > > smp_wmb(); /* commit the item before incrementing the head */ > > buffer->head = (head + 1) & (buffer->size - 1); > > /* wake_up() will make sure that the head is committed before > * waking anyone up */ > wake_up(consumer); > } > > spin_unlock(&producer_lock); We can see that authors of the document didn't put any memory barrier after "buffer->tail" read and before "produce_item(item)" and I think they have a good reason. Lets consider an imaginary smp_mb() right before "produce_item(item);". Such a barrier will ensure that - - the memory read on "buffer->tail" is completed before store to memory pointed by "item" is committed. However, the store to "buffer->tail" anyway cannot be completed before conditional branch implied by "if ()" is proven to execute body statement of the if(). And the latter cannot be proven before read of "buffer->tail" is completed. Lets see this other way. Lets imagine that somehow a store to the data pointed by "item" is completed before we read "buffer->tail". That would mean, that the store was completed speculatively. But speculative execution of conditional stores is prohibited by C/C++ standard, otherwise any conditional store at any random place of code could pollute shared memory. On the other hand, if compiler or processor can prove that condition in above if() is going to be true (or if speculative store writes the same value as it was before write), the speculative store *is* allowed. In this case we should not be bothered by the fact that memory pointed by "item" is written before a read from "buffer->tail" is completed. Regards, -- Victor