public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: David Howells <dhowells@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	linux-kernel@vger.kernel.org, Nicolas Pitre <nico@cam.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	benh@kernel.crashing.org, paulus@samba.org,
	David Miller <davem@davemloft.net>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arch@vger.kernel.org
Subject: Re: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()
Date: Sun, 9 Nov 2008 11:18:11 -0500	[thread overview]
Message-ID: <20081109161811.GA10181@Krystal> (raw)
In-Reply-To: <1219.1226231496@redhat.com>

* David Howells (dhowells@redhat.com) wrote:
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > > Note that that does not guarantee that the two reads will be done in the
> > > order you want.  The compiler barrier _only_ affects the compiler.  It
> > > does not stop the CPU from doing the reads in any order it wants.  You
> > > need something stronger than smp_rmb() if you need the reads to be so
> > > ordered.
> > 
> > For reading hardware devices that can indeed be correct. But for normal 
> > memory access on a uniprocessor, if the CPU were to reorder the reads that 
> > would effect the actual algorithm then that CPU is broken.
> > 
> >  read a
> >     <---  interrupt  - should see read a here before read b is done.
> >  read b
> 
> Life isn't that simple.  Go and read the section labelled "The things cpus get
> up to" in Documentation/memory-barriers.txt.
> 
> The two reads we're talking about are independent of each other.  Independent
> reads and writes can be reordered and merged at will by the CPU, subject to
> restrictions imposed by barriers, cacheability attributes, MMIO attributes and
> suchlike.
> 
> You can get read b happening before read a, but in such a case both
> instructions will be in the CPU's execution pipeline.  When an interrupt
> occurs, the CPU will presumably finish clearing what's in its pipeline before
> going and servicing the interrupt handler.
> 
> If a CPU is strictly ordered with respect to reads, do you actually need read
> barriers?
> 
> The fact that a pair of reads might be part of an algorithm that is critically
> dependent on the ordering of those reads isn't something the CPU cares about.
> It doesn't know there's an algorithm there.
> 
> > Now the fact that one of the reads is a hardware clock, then this 
> > statement might not be too strong. But the fact that it is a clock, and 
> > not some memory mapped device register, I still think smp_rmb is 
> > sufficient.
> 
> To quote again from memory-barriers.txt, section "CPU memory barriers":
> 
> 	Mandatory barriers should not be used to control SMP effects, since
> 	mandatory barriers unnecessarily impose overhead on UP systems. They
> 	may, however, be used to control MMIO effects on accesses through
> 	relaxed memory I/O windows.  These are required even on non-SMP
> 	systems 

<emphasis>
>   as they affect the order in which memory operations appear to a device
</emphasis>

In this particular case, we don't care about the order of memory
operations as seen by the device, given we only read the mmio time
source atomically. So considering what you said above about the fact
that the CPU will flush all the pending operations in the pipeline
before proceeding to service an interrupt, a simple barrier() should be
enough to make the two operations appear in correct order wrt local
interrupts. I therefore don't think a full rmb() is required to insure
correct read order on UP, because, again, in this case we don't need to
order accesses as seen by the device.

Mathieu

>   by prohibiting both the compiler and the CPU from reordering
> 	them.
> 
> Section "Accessing devices":
> 
>  (2) If the accessor functions are used to refer to an I/O memory window with
>      relaxed memory access properties, then _mandatory_ memory barriers are
>      required to enforce ordering.
> 
> David

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  parent reply	other threads:[~2008-11-09 16:24 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-07  5:23 [RFC patch 00/18] Trace Clock v2 Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 01/18] get_cycles() : kconfig HAVE_GET_CYCLES Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 02/18] get_cycles() : x86 HAVE_GET_CYCLES Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 03/18] get_cycles() : sparc64 HAVE_GET_CYCLES Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 04/18] get_cycles() : powerpc64 HAVE_GET_CYCLES Mathieu Desnoyers
2008-11-07 14:56   ` Josh Boyer
2008-11-07 18:14     ` Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 05/18] get_cycles() : MIPS HAVE_GET_CYCLES_32 Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 06/18] Trace clock generic Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 07/18] Trace clock core Mathieu Desnoyers
2008-11-07  5:52   ` Andrew Morton
2008-11-07  6:16     ` Mathieu Desnoyers
2008-11-07  6:26       ` Andrew Morton
2008-11-07 16:12         ` Mathieu Desnoyers
2008-11-07 16:19           ` Andrew Morton
2008-11-07 18:16             ` Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 08/18] cnt32_to_63 should use smp_rmb() Mathieu Desnoyers
2008-11-07  6:05   ` Andrew Morton
2008-11-07  8:12     ` Nicolas Pitre
2008-11-07  8:38       ` Andrew Morton
2008-11-07 11:20         ` David Howells
2008-11-07 15:01           ` Nicolas Pitre
2008-11-07 15:50             ` Andrew Morton
2008-11-07 16:21               ` David Howells
2008-11-07 16:29                 ` Andrew Morton
2008-11-07 17:10                   ` David Howells
2008-11-07 17:26                     ` Andrew Morton
2008-11-07 18:00                       ` Mathieu Desnoyers
2008-11-07 18:21                         ` Andrew Morton
2008-11-07 18:30                           ` Harvey Harrison
2008-11-07 18:42                             ` Mathieu Desnoyers
2008-11-07 18:33                           ` Mathieu Desnoyers
2008-11-07 18:36                           ` Linus Torvalds
2008-11-07 18:45                             ` Andrew Morton
2008-11-07 16:47               ` Nicolas Pitre
2008-11-07 16:55                 ` David Howells
2008-11-07 17:21                 ` Andrew Morton
2008-11-07 20:03                   ` Nicolas Pitre
2008-11-07 16:07             ` David Howells
2008-11-07 16:47           ` Mathieu Desnoyers
2008-11-07 17:04             ` David Howells
2008-11-07 17:17               ` Mathieu Desnoyers
2008-11-07 23:27                 ` David Howells
2008-11-07 20:11             ` Russell King
2008-11-07 21:36               ` Mathieu Desnoyers
2008-11-07 22:18                 ` Russell King
2008-11-07 22:36                   ` Mathieu Desnoyers
2008-11-07 23:41                   ` David Howells
2008-11-08  0:15                     ` Russell King
2008-11-08  0:45                       ` David Howells
2008-11-08 15:24                       ` Nicolas Pitre
2008-11-08 23:20                         ` [PATCH] clarify usage expectations for cnt32_to_63() Nicolas Pitre
2008-11-09  2:25                           ` Mathieu Desnoyers
2008-11-09  2:54                             ` Nicolas Pitre
2008-11-09  5:06                               ` Nicolas Pitre
2008-11-09  5:27                                 ` [PATCH v2] " Nicolas Pitre
2008-11-09  6:48                                   ` Mathieu Desnoyers
2008-11-09 13:34                                     ` Nicolas Pitre
2008-11-09 13:43                                       ` Russell King
2008-11-09 16:22                                       ` Mathieu Desnoyers
2008-11-10  4:20                                         ` Nicolas Pitre
2008-11-10  4:42                                           ` Andrew Morton
2008-11-10 21:34                                             ` Nicolas Pitre
2008-11-10 21:58                                               ` Andrew Morton
2008-11-10 23:15                                                 ` Nicolas Pitre
2008-11-10 23:22                                                   ` Andrew Morton
2008-11-10 23:38                                                     ` Steven Rostedt
2008-11-11  0:26                                                     ` Nicolas Pitre
2008-11-11 18:28                                                       ` [PATCH] convert cnt32_to_63 to inline Mathieu Desnoyers
2008-11-11 19:13                                                         ` Russell King
2008-11-11 20:11                                                           ` Mathieu Desnoyers
2008-11-11 21:51                                                             ` Russell King
2008-11-12  3:48                                                               ` Mathieu Desnoyers
2008-11-11 21:00                                                         ` Nicolas Pitre
2008-11-11 21:13                                                           ` Russell King
2008-11-11 22:31                                                         ` David Howells
2008-11-11 22:37                                                           ` Peter Zijlstra
2008-11-12  1:13                                                             ` Steven Rostedt
2008-11-07 11:03       ` [RFC patch 08/18] cnt32_to_63 should use smp_rmb() David Howells
2008-11-07 16:51         ` Mathieu Desnoyers
2008-11-07 20:18           ` Nicolas Pitre
2008-11-07 23:55             ` David Howells
2008-11-07 10:59     ` David Howells
2008-11-07 10:55   ` David Howells
2008-11-07 17:09     ` Mathieu Desnoyers
2008-11-07 17:33       ` Steven Rostedt
2008-11-07 19:18         ` Mathieu Desnoyers
2008-11-07 19:32           ` Peter Zijlstra
2008-11-07 20:02             ` Mathieu Desnoyers
2008-11-07 20:45               ` Mathieu Desnoyers
2008-11-07 20:54                 ` Paul E. McKenney
2008-11-07 21:04                   ` Steven Rostedt
2008-11-08  0:34                     ` Paul E. McKenney
2008-11-07 21:16                   ` Mathieu Desnoyers
2008-11-07 23:50         ` David Howells
2008-11-08  0:55           ` Steven Rostedt
2008-11-09 11:51             ` David Howells
2008-11-09 14:31               ` Steven Rostedt
2008-11-09 16:18               ` Mathieu Desnoyers [this message]
2008-11-07 20:08       ` Steven Rostedt
2008-11-07 20:55         ` Paul E. McKenney
2008-11-07 21:27         ` Mathieu Desnoyers
2008-11-07 20:36       ` Nicolas Pitre
2008-11-07 20:55         ` Mathieu Desnoyers
2008-11-07 21:22           ` Nicolas Pitre
2008-11-07  5:23 ` [RFC patch 09/18] Powerpc : Trace clock Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 10/18] Sparc64 " Mathieu Desnoyers
2008-11-07  5:45   ` David Miller
2008-11-07  5:23 ` [RFC patch 11/18] LTTng timestamp sh Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 12/18] LTTng - TSC synchronicity test Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 13/18] x86 : remove arch-specific tsc_sync.c Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 14/18] MIPS use tsc_sync.c Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 15/18] MIPS : export hpt frequency for trace_clock Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 16/18] MIPS create empty sync_core() Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 17/18] MIPS : Trace clock Mathieu Desnoyers
2008-11-07 11:53   ` Peter Zijlstra
2008-11-07 17:44     ` Mathieu Desnoyers
2008-11-07  5:23 ` [RFC patch 18/18] x86 trace clock Mathieu Desnoyers

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=20081109161811.GA10181@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=nico@cam.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=ralf@linux-mips.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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