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: Fri, 1 Nov 2013 15:12:58 +0200	[thread overview]
Message-ID: <OF6655E55D.554B7448-ON42257C16.00442B8B-42257C16.0048995E@il.ibm.com> (raw)
In-Reply-To: <20131031061602.GU4126@linux.vnet.ibm.com>

"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote on 10/31/2013
08:16:02 AM:

> > BTW, it is why you also don't need ACCESS_ONCE() around @tail, but only
> > around
> > @head read.

Just to be sure, that we are talking about the same code - I was
considering
ACCESS_ONCE() around @tail in point AAA in the following example from
Documentation/circular-buffers.txt for CONSUMER:

        unsigned long head = ACCESS_ONCE(buffer->head);
        unsigned long tail = buffer->tail;      /* AAA */

        if (CIRC_CNT(head, tail, buffer->size) >= 1) {
                /* read index before reading contents at that index */
                smp_read_barrier_depends();

                /* extract one item from the buffer */
                struct item *item = buffer[tail];

                consume_item(item);

                smp_mb(); /* finish reading descriptor before incrementing
tail */

                buffer->tail = (tail + 1) & (buffer->size - 1); /* BBB */
        }

>
> If you omit the ACCESS_ONCE() calls around @tail, the compiler is within
> its rights to combine adjacent operations and also to invent loads and
> stores, for example, in cases of register pressure.

Right. And I was completely aware about these possible transformations when
said that ACCESS_ONCE() around @tail in point AAA is redundant. Moved, or
even
completely dismissed reads of @tail in consumer code, are not a problem at
all,
since @tail is written exclusively by CONSUMER side.


> It is also within
> its rights to do piece-at-a-time loads and stores, which might sound
> unlikely, but which can actually has happened when the compiler figures
> out exactly what is to be stored at compile time, especially on hardware
> that only allows small immediate values.

As for writes to @tail, the ACCESS_ONCE around @tail at point AAA,
doesn't prevent in any way an imaginary super-optimizing compiler
from moving around the store to @tail (which appears in the code at point
BBB).

It is why ACCESS_ONCE at point AAA is completely redundant.

-- 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: Fri, 1 Nov 2013 15:12:58 +0200	[thread overview]
Message-ID: <OF6655E55D.554B7448-ON42257C16.00442B8B-42257C16.0048995E@il.ibm.com> (raw)
In-Reply-To: <20131031061602.GU4126@linux.vnet.ibm.com>

"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote on 10/31/2013
08:16:02 AM:

> > BTW, it is why you also don't need ACCESS_ONCE() around @tail, but only
> > around
> > @head read.

Just to be sure, that we are talking about the same code - I was
considering
ACCESS_ONCE() around @tail in point AAA in the following example from
Documentation/circular-buffers.txt for CONSUMER:

        unsigned long head = ACCESS_ONCE(buffer->head);
        unsigned long tail = buffer->tail;      /* AAA */

        if (CIRC_CNT(head, tail, buffer->size) >= 1) {
                /* read index before reading contents at that index */
                smp_read_barrier_depends();

                /* extract one item from the buffer */
                struct item *item = buffer[tail];

                consume_item(item);

                smp_mb(); /* finish reading descriptor before incrementing
tail */

                buffer->tail = (tail + 1) & (buffer->size - 1); /* BBB */
        }

>
> If you omit the ACCESS_ONCE() calls around @tail, the compiler is within
> its rights to combine adjacent operations and also to invent loads and
> stores, for example, in cases of register pressure.

Right. And I was completely aware about these possible transformations when
said that ACCESS_ONCE() around @tail in point AAA is redundant. Moved, or
even
completely dismissed reads of @tail in consumer code, are not a problem at
all,
since @tail is written exclusively by CONSUMER side.


> It is also within
> its rights to do piece-at-a-time loads and stores, which might sound
> unlikely, but which can actually has happened when the compiler figures
> out exactly what is to be stored at compile time, especially on hardware
> that only allows small immediate values.

As for writes to @tail, the ACCESS_ONCE around @tail at point AAA,
doesn't prevent in any way an imaginary super-optimizing compiler
from moving around the store to @tail (which appears in the code at point
BBB).

It is why ACCESS_ONCE at point AAA is completely redundant.

-- Victor

  reply	other threads:[~2013-11-01 13:13 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 [this message]
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
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=OF6655E55D.554B7448-ON42257C16.00442B8B-42257C16.0048995E@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.