Add a global synchronization point for pvclock
in [Kernel]
|
Prev: Sound not transferred to docking station after resume from StR
Next: Process-shared futexes on hugepages puts the kernel in an infinite loop in 2.6.32.11; is this fixed now?
From: Peter Zijlstra on 19 Apr 2010 07:10 On Mon, 2010-04-19 at 13:50 +0300, Avi Kivity wrote: > On 04/19/2010 01:39 PM, Peter Zijlstra wrote: > > On Fri, 2010-04-16 at 13:36 -0700, Jeremy Fitzhardinge wrote: > > > >>> + do { > >>> + last = last_value; > >>> > >>> > >> Does this need a barrier() to prevent the compiler from re-reading > >> last_value for the subsequent lines? Otherwise "(ret< last)" and > >> "return last" could execute with different values for "last". > > > ACCESS_ONCE() is your friend. > > > > I think it's implied with atomic64_read(). Yes it would be. I was merely trying to point out that last = ACCESS_ONCE(last_value); Is a narrower way of writing: last = last_value; barrier(); In that it need not clobber all memory locations and makes it instantly clear what we want the barrier for. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 19 Apr 2010 07:20 On 04/19/2010 02:05 PM, Peter Zijlstra wrote: >> >>> ACCESS_ONCE() is your friend. >>> >>> >> I think it's implied with atomic64_read(). >> > Yes it would be. I was merely trying to point out that > > last = ACCESS_ONCE(last_value); > > Is a narrower way of writing: > > last = last_value; > barrier(); > > In that it need not clobber all memory locations and makes it instantly > clear what we want the barrier for. > Oh yes, just trying to avoid a patch with both atomic64_read() and ACCESS_ONCE(). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 19 Apr 2010 07:20 On Mon, 2010-04-19 at 14:13 +0300, Avi Kivity wrote: > On 04/19/2010 01:56 PM, Peter Zijlstra wrote: > > > > > >>> Right, do bear in mind that the x86 implementation of atomic64_read() is > >>> terrifyingly expensive, it is better to not do that read and simply use > >>> the result of the cmpxchg. > >>> > >>> > >>> > >> atomic64_read() _is_ cmpxchg64b. Are you thinking of some clever > >> implementation for smp i386? > >> > > > > No, what I was suggesting was to rewrite that loop no to need the > > initial read but use the cmpxchg result of the previous iteration. > > > > Something like: > > > > u64 last = 0; > > > > /* more stuff */ > > > > do { > > if (ret< last) > > return last; > > last = cmpxchg64(&last_value, last, ret); > > } while (last != ret); > > > > That only has a single cmpxchg8 in there per loop instead of two > > (avoiding the atomic64_read() one). > > > > Still have two cmpxchgs in the common case. The first iteration will > fail, fetching last_value, the second will work. > > It will be better when we have contention, though, so it's worthwhile. Right, another option is to put the initial read outside of the loop, that way you'll have the best of all cases, a single LOCK'ed op in the loop, and only a single LOCK'ed op for the fast path on sensible architectures ;-) last = atomic64_read(&last_value); do { if (ret < last) return last; last = atomic64_cmpxchg(&last_value, last, ret); } while (unlikely(last != ret)); Or so. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 19 Apr 2010 07:20 On 04/19/2010 01:56 PM, Peter Zijlstra wrote: > > >>> Right, do bear in mind that the x86 implementation of atomic64_read() is >>> terrifyingly expensive, it is better to not do that read and simply use >>> the result of the cmpxchg. >>> >>> >>> >> atomic64_read() _is_ cmpxchg64b. Are you thinking of some clever >> implementation for smp i386? >> > > No, what I was suggesting was to rewrite that loop no to need the > initial read but use the cmpxchg result of the previous iteration. > > Something like: > > u64 last = 0; > > /* more stuff */ > > do { > if (ret< last) > return last; > last = cmpxchg64(&last_value, last, ret); > } while (last != ret); > > That only has a single cmpxchg8 in there per loop instead of two > (avoiding the atomic64_read() one). > Still have two cmpxchgs in the common case. The first iteration will fail, fetching last_value, the second will work. It will be better when we have contention, though, so it's worthwhile. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 19 Apr 2010 07:40
On 04/19/2010 01:59 PM, Peter Zijlstra wrote: > >> So what do we need? test for both TSC_RELIABLE and NONSTOP_TSC? IMO >> TSC_RELIABLE should imply NONSTOP_TSC. >> > Yeah, I think RELIABLE does imply NONSTOP and CONSTANT, but NONSTOP&& > CONSTANT does not make RELIABLE. > The manual says: > 16.11.1 Invariant TSC > > The time stamp counter in newer processors may support an enhancement, > referred > to as invariant TSC. Processor's support for invariant TSC is indicated by > CPUID.80000007H:EDX[8]. > The invariant TSC will run at a constant rate in all ACPI P-, C-. and > T-states. This is > the architectural behavior moving forward. On processors with > invariant TSC > support, the OS may use the TSC for wall clock timer services (instead > of ACPI or > HPET timers). TSC reads are much more efficient and do not incur the > overhead > associated with a ring transition or access to a platform resource. and this maps to NONSTOP, so I think we're fine. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |