All of lore.kernel.org
 help / color / mirror / Atom feed
From: Victor Kaplansky <VICTORK@il.ibm.com>
To: paulmck@linux.vnet.ibm.com
Cc: Anton Blanchard <anton@samba.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux PPC dev <linuxppc-dev@ozlabs.org>,
	Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	Michael Ellerman <michael@ellerman.id.au>,
	Michael Neuling <mikey@neuling.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: perf events ring buffer memory barrier on powerpc
Date: Wed, 30 Oct 2013 15:28:54 +0200	[thread overview]
Message-ID: <OFC6B7100E.31C6B190-ON42257C14.004841C6-42257C14.004A0EA6@il.ibm.com> (raw)
In-Reply-To: <20131030092725.GL4126@linux.vnet.ibm.com>

"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> 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


WARNING: multiple messages have this Message-ID (diff)
From: Victor Kaplansky <VICTORK@il.ibm.com>
To: paulmck@linux.vnet.ibm.com
Cc: Michael Neuling <mikey@neuling.org>,
	Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Linux PPC dev <linuxppc-dev@ozlabs.org>,
	Anton Blanchard <anton@samba.org>,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: perf events ring buffer memory barrier on powerpc
Date: Wed, 30 Oct 2013 15:28:54 +0200	[thread overview]
Message-ID: <OFC6B7100E.31C6B190-ON42257C14.004841C6-42257C14.004A0EA6@il.ibm.com> (raw)
In-Reply-To: <20131030092725.GL4126@linux.vnet.ibm.com>

"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> 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

  parent reply	other threads:[~2013-10-30 13:30 UTC|newest]

Thread overview: 215+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-22 23:54 perf events ring buffer memory barrier on powerpc Michael Neuling
2013-10-23  7:39 ` Victor Kaplansky
2013-10-23  7:39   ` Victor Kaplansky
2013-10-23 14:19 ` Frederic Weisbecker
2013-10-23 14:19   ` Frederic Weisbecker
2013-10-23 14:25   ` Frederic Weisbecker
2013-10-23 14:25     ` Frederic Weisbecker
2013-10-25 17:37   ` Peter Zijlstra
2013-10-25 17:37     ` Peter Zijlstra
2013-10-25 20:31     ` Michael Neuling
2013-10-25 20:31       ` Michael Neuling
2013-10-27  9:00     ` Victor Kaplansky
2013-10-27  9:00       ` Victor Kaplansky
2013-10-28  9:22       ` Peter Zijlstra
2013-10-28  9:22         ` Peter Zijlstra
2013-10-28 10:02     ` Frederic Weisbecker
2013-10-28 10:02       ` Frederic Weisbecker
2013-10-28 12:38       ` Victor Kaplansky
2013-10-28 12:38         ` Victor Kaplansky
2013-10-28 13:26         ` Peter Zijlstra
2013-10-28 13:26           ` Peter Zijlstra
2013-10-28 16:34           ` Paul E. McKenney
2013-10-28 16:34             ` Paul E. McKenney
2013-10-28 20:17             ` Oleg Nesterov
2013-10-28 20:17               ` Oleg Nesterov
2013-10-28 20:58               ` Victor Kaplansky
2013-10-28 20:58                 ` Victor Kaplansky
2013-10-29 10:21                 ` Peter Zijlstra
2013-10-29 10:21                   ` Peter Zijlstra
2013-10-29 10:30                   ` Peter Zijlstra
2013-10-29 10:30                     ` Peter Zijlstra
2013-10-29 10:35                     ` Peter Zijlstra
2013-10-29 10:35                       ` Peter Zijlstra
2013-10-29 20:15                       ` Oleg Nesterov
2013-10-29 20:15                         ` Oleg Nesterov
2013-10-29 19:27                     ` Vince Weaver
2013-10-29 19:27                       ` Vince Weaver
2013-10-30 10:42                       ` Peter Zijlstra
2013-10-30 10:42                         ` Peter Zijlstra
2013-10-30 11:48                         ` James Hogan
2013-10-30 11:48                           ` James Hogan
2013-10-30 12:48                           ` Peter Zijlstra
2013-10-30 12:48                             ` Peter Zijlstra
2013-11-06 13:19                         ` [tip:perf/core] tools/perf: Add required memory barriers tip-bot for Peter Zijlstra
2013-11-06 13:50                           ` Vince Weaver
2013-11-06 14:00                             ` Peter Zijlstra
2013-11-06 14:28                               ` Peter Zijlstra
2013-11-06 14:55                                 ` Vince Weaver
2013-11-06 15:10                                   ` Peter Zijlstra
2013-11-06 15:23                                     ` Peter Zijlstra
2013-11-06 14:44                               ` Peter Zijlstra
2013-11-06 16:07                                 ` Peter Zijlstra
2013-11-06 17:31                                   ` Vince Weaver
2013-11-06 18:24                                     ` Peter Zijlstra
2013-11-07  8:21                                       ` Ingo Molnar
2013-11-07 14:27                                         ` Vince Weaver
2013-11-07 15:55                                           ` Ingo Molnar
2013-11-11 16:24                                         ` Peter Zijlstra
2013-11-11 21:10                                           ` Ingo Molnar
2013-10-29 21:23                     ` perf events ring buffer memory barrier on powerpc Michael Neuling
2013-10-29 21:23                       ` Michael Neuling
2013-10-30  9:27                 ` Paul E. McKenney
2013-10-30  9:27                   ` Paul E. McKenney
2013-10-30 11:25                   ` Peter Zijlstra
2013-10-30 11:25                     ` Peter Zijlstra
2013-10-30 14:52                     ` Victor Kaplansky
2013-10-30 14:52                       ` Victor Kaplansky
2013-10-30 15:39                       ` Peter Zijlstra
2013-10-30 15:39                         ` Peter Zijlstra
2013-10-30 17:14                         ` Victor Kaplansky
2013-10-30 17:14                           ` Victor Kaplansky
2013-10-30 17:44                           ` Peter Zijlstra
2013-10-30 17:44                             ` Peter Zijlstra
2013-10-31  6:16                       ` Paul E. McKenney
2013-10-31  6:16                         ` Paul E. McKenney
2013-11-01 13:12                         ` Victor Kaplansky
2013-11-01 13:12                           ` Victor Kaplansky
2013-11-02 16:36                           ` Paul E. McKenney
2013-11-02 16:36                             ` Paul E. McKenney
2013-11-02 17:26                             ` Paul E. McKenney
2013-11-02 17:26                               ` Paul E. McKenney
2013-10-31  6:40                     ` Paul E. McKenney
2013-10-31  6:40                       ` Paul E. McKenney
2013-11-01 14:25                       ` Victor Kaplansky
2013-11-01 14:25                         ` Victor Kaplansky
2013-11-02 17:28                         ` Paul E. McKenney
2013-11-02 17:28                           ` Paul E. McKenney
2013-11-01 14:56                       ` Peter Zijlstra
2013-11-01 14:56                         ` Peter Zijlstra
2013-11-02 17:32                         ` Paul E. McKenney
2013-11-02 17:32                           ` Paul E. McKenney
2013-11-03 14:40                           ` Paul E. McKenney
2013-11-03 14:40                             ` Paul E. McKenney
2013-11-03 15:17                             ` [RFC] arch: Introduce new TSO memory barrier smp_tmb() Peter Zijlstra
2013-11-03 15:17                               ` Peter Zijlstra
2013-11-03 18:08                               ` Linus Torvalds
2013-11-03 18:08                                 ` Linus Torvalds
2013-11-03 20:01                                 ` Peter Zijlstra
2013-11-03 20:01                                   ` Peter Zijlstra
2013-11-03 22:42                                   ` Paul E. McKenney
2013-11-03 22:42                                     ` Paul E. McKenney
2013-11-03 23:34                                     ` Linus Torvalds
2013-11-03 23:34                                       ` Linus Torvalds
2013-11-04 10:51                                       ` Paul E. McKenney
2013-11-04 10:51                                         ` Paul E. McKenney
2013-11-04 11:22                                         ` Peter Zijlstra
2013-11-04 11:22                                           ` Peter Zijlstra
2013-11-04 16:27                                           ` Paul E. McKenney
2013-11-04 16:27                                             ` Paul E. McKenney
2013-11-04 16:48                                             ` Peter Zijlstra
2013-11-04 16:48                                               ` Peter Zijlstra
2013-11-04 19:11                                             ` Peter Zijlstra
2013-11-04 19:11                                               ` Peter Zijlstra
2013-11-04 19:18                                               ` Peter Zijlstra
2013-11-04 19:18                                                 ` Peter Zijlstra
2013-11-04 20:54                                                 ` Paul E. McKenney
2013-11-04 20:54                                                   ` Paul E. McKenney
2013-11-04 20:53                                               ` Paul E. McKenney
2013-11-04 20:53                                                 ` Paul E. McKenney
2013-11-05 14:05                                                 ` Will Deacon
2013-11-05 14:05                                                   ` Will Deacon
2013-11-05 14:49                                                   ` Paul E. McKenney
2013-11-05 14:49                                                     ` Paul E. McKenney
2013-11-05 18:49                                                   ` Peter Zijlstra
2013-11-05 18:49                                                     ` Peter Zijlstra
2013-11-06 11:00                                                     ` Will Deacon
2013-11-06 11:00                                                       ` Will Deacon
2013-11-06 12:39                                                 ` Peter Zijlstra
2013-11-06 12:39                                                   ` Peter Zijlstra
2013-11-06 12:51                                                   ` Geert Uytterhoeven
2013-11-06 12:51                                                     ` Geert Uytterhoeven
2013-11-06 13:57                                                     ` Peter Zijlstra
2013-11-06 13:57                                                       ` Peter Zijlstra
2013-11-06 18:48                                                       ` Paul E. McKenney
2013-11-06 18:48                                                         ` Paul E. McKenney
2013-11-06 19:42                                                         ` Peter Zijlstra
2013-11-06 19:42                                                           ` Peter Zijlstra
2013-11-07 11:17                                                       ` Will Deacon
2013-11-07 11:17                                                         ` Will Deacon
2013-11-07 13:36                                                         ` Peter Zijlstra
2013-11-07 13:36                                                           ` Peter Zijlstra
2013-11-07 23:50                                           ` Mathieu Desnoyers
2013-11-07 23:50                                             ` Mathieu Desnoyers
2013-11-04 11:05                                       ` Will Deacon
2013-11-04 11:05                                         ` Will Deacon
2013-11-04 16:34                                         ` Paul E. McKenney
2013-11-04 16:34                                           ` Paul E. McKenney
2013-11-03 20:59                               ` Benjamin Herrenschmidt
2013-11-03 20:59                                 ` Benjamin Herrenschmidt
2013-11-03 22:43                                 ` Paul E. McKenney
2013-11-03 22:43                                   ` Paul E. McKenney
2013-11-03 17:07                             ` perf events ring buffer memory barrier on powerpc 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-01 16:11                         ` Peter Zijlstra
2013-11-02 17:46                         ` Paul E. McKenney
2013-11-02 17:46                           ` Paul E. McKenney
2013-11-01 16:18                       ` Peter Zijlstra
2013-11-01 16:18                         ` Peter Zijlstra
2013-11-02 17:49                         ` Paul E. McKenney
2013-11-02 17:49                           ` Paul E. McKenney
2013-10-30 13:28                   ` Victor Kaplansky [this message]
2013-10-30 13:28                     ` Victor Kaplansky
2013-10-30 15:51                     ` Peter Zijlstra
2013-10-30 15:51                       ` Peter Zijlstra
2013-10-30 18:29                       ` Peter Zijlstra
2013-10-30 18:29                         ` Peter Zijlstra
2013-10-30 19:11                         ` Peter Zijlstra
2013-10-30 19:11                           ` Peter Zijlstra
2013-10-31  4:33                       ` Paul E. McKenney
2013-10-31  4:33                         ` Paul E. McKenney
2013-10-31  4:32                     ` Paul E. McKenney
2013-10-31  4:32                       ` Paul E. McKenney
2013-10-31  9:04                       ` Peter Zijlstra
2013-10-31  9:04                         ` Peter Zijlstra
2013-10-31 15:07                         ` Paul E. McKenney
2013-10-31 15:07                           ` Paul E. McKenney
2013-10-31 15:19                           ` Peter Zijlstra
2013-10-31 15:19                             ` Peter Zijlstra
2013-11-01  9:28                             ` Paul E. McKenney
2013-11-01  9:28                               ` Paul E. McKenney
2013-11-01 10:30                               ` Peter Zijlstra
2013-11-01 10:30                                 ` Peter Zijlstra
2013-11-02 15:20                                 ` Paul E. McKenney
2013-11-02 15:20                                   ` Paul E. McKenney
2013-11-04  9:07                                   ` Peter Zijlstra
2013-11-04  9:07                                     ` Peter Zijlstra
2013-11-04 10:00                                     ` Paul E. McKenney
2013-11-04 10:00                                       ` Paul E. McKenney
2013-10-31  9:59                       ` Victor Kaplansky
2013-10-31  9:59                         ` Victor Kaplansky
2013-10-31 12:28                         ` David Laight
2013-10-31 12:28                           ` David Laight
2013-10-31 12:55                           ` Victor Kaplansky
2013-10-31 12:55                             ` Victor Kaplansky
2013-10-31 15:25                         ` Paul E. McKenney
2013-10-31 15:25                           ` Paul E. McKenney
2013-11-01 16:06                           ` Victor Kaplansky
2013-11-01 16:06                             ` Victor Kaplansky
2013-11-01 16:25                             ` David Laight
2013-11-01 16:25                               ` David Laight
2013-11-01 16:30                               ` Victor Kaplansky
2013-11-01 16:30                                 ` Victor Kaplansky
2013-11-03 20:57                                 ` Benjamin Herrenschmidt
2013-11-03 20:57                                   ` Benjamin Herrenschmidt
2013-11-02 15:46                             ` Paul E. McKenney
2013-11-02 15:46                               ` Paul E. McKenney
2013-10-28 19:09           ` Oleg Nesterov
2013-10-28 19:09             ` Oleg Nesterov
2013-10-29 14:06     ` [tip:perf/urgent] perf: Fix perf ring buffer memory ordering tip-bot for Peter Zijlstra
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

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=OFC6B7100E.31C6B190-ON42257C14.004841C6-42257C14.004A0EA6@il.ibm.com \
    --to=victork@il.ibm.com \
    --cc=anton@samba.org \
    --cc=benh@kernel.crashing.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=michael@ellerman.id.au \
    --cc=mikey@neuling.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.