linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Victor Kaplansky <VICTORK@il.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org
Subject: Re: perf events ring buffer memory barrier on powerpc
Date: Fri, 9 May 2014 06:47:26 -0700	[thread overview]
Message-ID: <20140509134726.GR8754@linux.vnet.ibm.com> (raw)
In-Reply-To: <alpine.LRH.2.02.1405090755320.21432@file01.intranet.prod.int.rdu2.redhat.com>

On Fri, May 09, 2014 at 08:20:25AM -0400, Mikulas Patocka wrote:
> 
> 
> On Fri, 9 May 2014, Victor Kaplansky wrote:
> 
> > Mikulas Patocka <mpatocka@redhat.com> wrote on 05/08/2014 11:46:53 PM:
> > 
> > > > > BTW, it is why you also don't need ACCESS_ONCE() around @tail, but only
> > > > > around
> > > > > @head read.
> > > >
> > > > Agreed, the ACCESS_ONCE() around tail is superfluous since we're the one
> > > > updating tail, so there's no problem with the value changing
> > > > unexpectedly.
> > >
> > > You need ACCESS_ONCE even if you are the only process writing the value.
> > > Because without ACCESS_ONCE, the compiler may perform store tearing and
> > > split the store into several smaller stores. Search the file
> > > "Documentation/memory-barriers.txt" for the term "store tearing", it shows
> > > an example where one instruction storing 32-bit value may be split to two
> > > instructions, each storing 16-bit value.
> > >
> > > Mikulas
> > 
> > AFAIR, I was talking about redundant ACCESS_ONCE() around @tail *read* in
> > consumer code. As for ACCESS_ONCE() around @tail write in consumer code,
> > I see your point, but I don't think that volatile imposed by ACCESS_ONCE()
> > is appropriate, since:
> > 
> >     - compiler can generate several stores despite volatile if @tail
> >     is bigger in size than native machine data size, e.g. 64-bit on
> >     a 32-bit CPU.
> 
> That's true - so you should define data_head and data_tail as "unsigned 
> long", not "__u64".
> 
> >     - volatile imposed by ACCESS_ONCE() does nothing to prevent CPU from
> >     reordering, splitting or merging accesses. It can only mediate
> >     communication problems between processes running on same CPU.
> 
> That's why you need smp barrier in addition to ACCESS_ONCE. You need both 
> - the smp barrier (to prevent the CPU from reordering) and ACCESS_ONCE (to 
> prevent the compiler from splitting the write to smaller memory accesses).

IIRC the ring-buffer code uses the fact that one element remains
empty to make clever double use of a memory barrier.

> Since Linux 3.14, there are new macros smp_store_release and 
> smp_load_acquire that combine ACCESS_ONCE and memory barrier, so you can 
> use them. (they call compiletime_assert_atomic_type to make sure that you 
> don't use them on types that are not atomic, such as long long on 32-bit 
> architectures)

These are indeed useful and often simpler to use than raw barriers.

							Thanx, Paul

> > What you really want is to guarantee *atomicity* of @tail write on consumer
> > side.
> > 
> > -- Victor
> 
> Mikulas


  reply	other threads:[~2014-05-09 13:47 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-08 20:46 perf events ring buffer memory barrier on powerpc Mikulas Patocka
     [not found] ` <OF667059AA.7F151BCC-ONC2257CD3.0036CFEB-C2257CD3.003BBF01@il.ibm.com>
2014-05-09 12:20   ` Mikulas Patocka
2014-05-09 13:47     ` Paul E. McKenney [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-10-22 23:54 Michael Neuling
2013-10-23  7:39 ` Victor Kaplansky
2013-10-23 14:19 ` Frederic Weisbecker
2013-10-23 14:25   ` Frederic Weisbecker
2013-10-25 17:37   ` Peter Zijlstra
2013-10-25 20:31     ` Michael Neuling
2013-10-27  9:00     ` Victor Kaplansky
2013-10-28  9:22       ` Peter Zijlstra
2013-10-28 10:02     ` Frederic Weisbecker
2013-10-28 12:38       ` Victor Kaplansky
2013-10-28 13:26         ` Peter Zijlstra
2013-10-28 16:34           ` Paul E. McKenney
2013-10-28 20:17             ` Oleg Nesterov
2013-10-28 20:58               ` Victor Kaplansky
2013-10-29 10:21                 ` Peter Zijlstra
2013-10-29 10:30                   ` Peter Zijlstra
2013-10-29 10:35                     ` Peter Zijlstra
2013-10-29 20:15                       ` Oleg Nesterov
2013-10-29 19:27                     ` Vince Weaver
2013-10-30 10:42                       ` Peter Zijlstra
2013-10-30 11:48                         ` James Hogan
2013-10-30 12:48                           ` Peter Zijlstra
2013-10-29 21:23                     ` Michael Neuling
2013-10-30  9:27                 ` Paul E. McKenney
2013-10-30 11:25                   ` Peter Zijlstra
2013-10-30 14:52                     ` Victor Kaplansky
2013-10-30 15:39                       ` Peter Zijlstra
2013-10-30 17:14                         ` Victor Kaplansky
2013-10-30 17:44                           ` Peter Zijlstra
2013-10-31  6:16                       ` Paul E. McKenney
2013-11-01 13:12                         ` Victor Kaplansky
2013-11-02 16:36                           ` Paul E. McKenney
2013-11-02 17:26                             ` Paul E. McKenney
2013-10-31  6:40                     ` Paul E. McKenney
2013-11-01 14:25                       ` Victor Kaplansky
2013-11-02 17:28                         ` Paul E. McKenney
2013-11-01 14:56                       ` Peter Zijlstra
2013-11-02 17:32                         ` Paul E. McKenney
2013-11-03 14:40                           ` Paul E. McKenney
2013-11-03 17:07                             ` Will Deacon
2013-11-03 22:47                               ` Paul E. McKenney
2013-11-04  9:57                                 ` Will Deacon
2013-11-04 10:52                                   ` Paul E. McKenney
2013-11-01 16:11                       ` Peter Zijlstra
2013-11-02 17:46                         ` Paul E. McKenney
2013-11-01 16:18                       ` Peter Zijlstra
2013-11-02 17:49                         ` Paul E. McKenney
2013-10-30 13:28                   ` Victor Kaplansky
2013-10-30 15:51                     ` Peter Zijlstra
2013-10-30 18:29                       ` Peter Zijlstra
2013-10-30 19:11                         ` Peter Zijlstra
2013-10-31  4:33                       ` Paul E. McKenney
2013-10-31  4:32                     ` Paul E. McKenney
2013-10-31  9:04                       ` Peter Zijlstra
2013-10-31 15:07                         ` Paul E. McKenney
2013-10-31 15:19                           ` Peter Zijlstra
2013-11-01  9:28                             ` Paul E. McKenney
2013-11-01 10:30                               ` Peter Zijlstra
2013-11-02 15:20                                 ` Paul E. McKenney
2013-11-04  9:07                                   ` Peter Zijlstra
2013-11-04 10:00                                     ` Paul E. McKenney
2013-10-31  9:59                       ` Victor Kaplansky
2013-10-31 12:28                         ` David Laight
2013-10-31 12:55                           ` Victor Kaplansky
2013-10-31 15:25                         ` Paul E. McKenney
2013-11-01 16:06                           ` Victor Kaplansky
2013-11-01 16:25                             ` David Laight
2013-11-01 16:30                               ` Victor Kaplansky
2013-11-03 20:57                                 ` Benjamin Herrenschmidt
2013-11-02 15:46                             ` Paul E. McKenney
2013-10-28 19:09           ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140509134726.GR8754@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=VICTORK@il.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).