On Mon, 2010-04-19 at 13:47 +0300, Avi Kivity wrote:
> On 04/19/2010 01:43 PM, Peter Zijlstra wrote:
> >
> >>>
> >>>> +
> >>>> cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
> >>>> {
> >>>> struct pvclock_shadow_time shadow;
> >>>> unsigned version;
> >>>> cycle_t ret, offset;
> >>>> + u64 last;
> >>>>
> >>>>
> >>>> + do {
> >>>> + last = last_value;
> >>>>
> >>> Otherwise, this assignment can see a partial update.
> >>>
> >> On a 32-bit guest, that is.
> >>
> > 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).
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
