[RFC patch 08/18] cnt32_to_63 should use smp_rmb()

[Mathieu Desnoyers]

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@xxxxxxxxxx>
CC: Nicolas Pitre <nico@xxxxxxx>
CC: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
CC: benh@xxxxxxxxxxxxxxxxxxx
CC: paulus@xxxxxxxxx
CC: David Miller <davem@xxxxxxxxxxxxx>
CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
CC: Ingo Molnar <mingo@xxxxxxxxxx>
CC: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
CC: linux-arch@xxxxxxxxxxxxxxx
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/