On Thu, Nov 01, 2012 at 06:04:02PM +0400, Glauber Costa wrote:
> On 11/01/2012 02:47 AM, Marcelo Tosatti wrote:
> > +static __always_inline
> > +unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
> > + cycle_t *cycles, u8 *flags)
> > +{
> > + unsigned version;
> > + cycle_t ret, offset;
> > + u8 ret_flags;
> > +
> > + version = src->version;
> > + rdtsc_barrier();
> > + offset = pvclock_get_nsec_offset(src);
> > + ret = src->system_time + offset;
> > + ret_flags = src->flags;
> > + rdtsc_barrier();
> > +
> > + *cycles = ret;
> > + *flags = ret_flags;
> > + return version;
> > +}
> > +
> This interface is a bit weird.
> The actual value you are interested in is "cycles", so why is it
> returned through the parameters? I think it would be clearer to have
> this return cycles, and &version as a parameter.
I disagree because
do {
version = pvclock_read_cycles();
} while (version != src->version);
Looks fine.
--
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
