lkml.org 
[lkml]   [2008]   [Nov]   [7]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
DateFri, 07 Nov 2008 00:23:44 -0500
FromMathieu Desnoyers <>
Subject[RFC patch 08/18] cnt32_to_63 should use smp_rmb()
Assume the time source is a global clock which insures that time will never
*ever* go backward. Use a smp_rmb() to make sure the cnt_lo value is read before
__m_cnt_hi.

Remove the now unnecessary volatile statement. The barrier takes care of memory
ordering.

Mathieu:
> Yup, you are right. However, the case where one CPU sees the clock source
> a little bit off-sync (late) still poses a problem. Example follows :
>
> CPU    A                                 B
>        read __m_cnt_hi (0x80000000)
>        read hw cnt low (0x00000001)
>        (wrap detected :
>         (s32)(0x80000000 ^ 0x1) < 0)
>        write __m_cnt_hi = 0x00000001
>        return 0x0000000100000001
>                                             read __m_cnt_hi (0x00000001)
>                                      (late) read hw cnt low (0xFFFFFFFA)
>                                             (wrap detected :
>                                              (s32)(0x00000001 ^ 0xFFFFFFFA) <
+0)
>                                             write __m_cnt_hi = 0x80000001
>                                             return 0x80000001FFFFFFFA
>                                                                   (time jumps)
> A similar situation can be generated by out-of-order hi/low bits reads.

Nicolas:
This, of course, should and can be prevented.  No big deal.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Nicolas Pitre <nico@cam.org>
CC: Ralf Baechle <ralf@linux-mips.org>
CC: benh@kernel.crashing.org
CC: paulus@samba.org
CC: David Miller <davem@davemloft.net>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Ingo Molnar <mingo@redhat.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: linux-arch@vger.kernel.org
---
 include/linux/cnt32_to_63.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6-lttng/include/linux/cnt32_to_63.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/cnt32_to_63.h	2008-11-04 01:39:03.000000000 -0500
+++ linux-2.6-lttng/include/linux/cnt32_to_63.h	2008-11-04 01:48:50.000000000 -0500
@@ -65,12 +65,17 @@ union cnt32_to_63 {
  * implicitly by making the multiplier even, therefore saving on a runtime
  * clear-bit instruction. Otherwise caller must remember to clear the top
  * bit explicitly.
+ *
+ * Assume the time source is a global clock read from memory mapped I/O which
+ * insures that time will never *ever* go backward. Using a smp_rmb() to make
+ * sure the __m_cnt_hi value is read before the cnt_lo mmio read.
  */
 #define cnt32_to_63(cnt_lo) \
 ({ \
-	static volatile u32 __m_cnt_hi; \
+	static u32 __m_cnt_hi; \
 	union cnt32_to_63 __x; \
 	__x.hi = __m_cnt_hi; \
+	smp_rmb(); 	/* read __m_cnt_hi before mmio cnt_lo */ \
 	__x.lo = (cnt_lo); \
 	if (unlikely((s32)(__x.hi ^ __x.lo) < 0)) \
 		__m_cnt_hi = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); \
-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68


\
 
 \ /
  Last update: 2008-11-07 06:53    [from the cache]
©2003-2008