* David Howells (dhowells@xxxxxxxxxx) wrote: > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxx> wrote: > > > 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. > > If you have an smp_rmb(), then don't you need an smp_wmb()/smp_mb() to match > it to make it work? And is your assumption valid that smp_rmb() will affect > memory vs the I/O access to read the clock? There's no requirement that > cnt_lo will have been read from an MMIO location at all. > > David I want to make sure __m_cnt_hi is read before mmio cnt_lo read for the detailed reasons explained in my previous discussion with Nicolas here : http://lkml.org/lkml/2008/10/21/1 I use smp_rmb() to do this on SMP systems (hrm, actually, a rmb() could be required so it works also on UP systems safely wrt interrupts). The write side is between the hardware counter, which is assumed to increment monotonically between each read, and the value __m_cnt_hi updated by the CPU. I don't see where we could put a wmb() there. Without barrier, the smp race looks as follow : CPU A B read hw cnt low (0xFFFFFFFA) read __m_cnt_hi (0x80000000) read hw cnt low (0x00000001) (wrap detected : (s32)(0x80000000 ^ 0x1) < 0) write __m_cnt_hi = 0x00000001 read __m_cnt_hi (0x00000001) return 0x0000000100000001 (wrap detected : (s32)(0x00000001 ^ 0xFFFFFFFA) < 0) write __m_cnt_hi = 0x80000001 return 0x80000001FFFFFFFA (time jumps) And UP interrupt race : Thread context Interrupts read hw cnt low (0xFFFFFFFA) read __m_cnt_hi (0x80000000) read hw cnt low (0x00000001) (wrap detected : (s32)(0x80000000 ^ 0x1) < 0) write __m_cnt_hi = 0x00000001 read __m_cnt_hi (0x00000001) return 0x0000000100000001 (wrap detected : (s32)(0x00000001 ^ 0xFFFFFFFA) < 0) write __m_cnt_hi = 0x80000001 return 0x80000001FFFFFFFA (time jumps) New code to fix it here with full rmb() : static inline u64 cnt32_to_63(u32 io_addr, u32 *__m_cnt_hi) { union cnt32_to_63 __x; __x.hi = *__m_cnt_hi; /* memory read for high bits internal state */ rmb(); /* * read high bits before low bits insures time * does not go backward. Sync across * CPUs and for interrupts. */ __x.lo = readl(cnt_lo); /* mmio read */ if (unlikely((s32)(__x.hi ^ __x.lo) < 0)) *__m_cnt_hi = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); return __x.val; } Mathieu -- 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-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
