Re: [patch 04/16] x86: pvclock: create helper for pvclock data retrieval

2012-11-01 Thread Glauber Costa 
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.


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 04/16] x86: pvclock: create helper for pvclock data retrieval

2012-11-01 Thread Marcelo Tosatti 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 04/16] x86: pvclock: create helper for pvclock data retrieval

2012-10-31 Thread Marcelo Tosatti 
Originally from Jeremy Fitzhardinge.

So code can be reused.

Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

Index: vsyscall/arch/x86/kernel/pvclock.c
===
--- vsyscall.orig/arch/x86/kernel/pvclock.c
+++ vsyscall/arch/x86/kernel/pvclock.c
@@ -26,13 +26,6 @@ void pvclock_set_flags(u8 flags)
valid_flags = flags;
 }
 
-static u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
-{
-   u64 delta = native_read_tsc() - src-tsc_timestamp;
-   return pvclock_scale_delta(delta, src-tsc_to_system_mul,
-  src-tsc_shift);
-}
-
 unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src)
 {
u64 pv_tsc_khz = 100ULL  32;
@@ -55,17 +48,12 @@ void pvclock_resume(void)
 cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
 {
unsigned version;
-   cycle_t ret, offset;
+   cycle_t ret;
u64 last;
u8 flags;
 
do {
-   version = src-version;
-   rdtsc_barrier();
-   offset = pvclock_get_nsec_offset(src);
-   ret = src-system_time + offset;
-   flags = src-flags;
-   rdtsc_barrier();
+   version = __pvclock_read_cycles(src, ret, flags);
} while ((src-version  1) || version != src-version);
 
if ((valid_flags  PVCLOCK_TSC_STABLE_BIT) 
Index: vsyscall/arch/x86/include/asm/pvclock.h
===
--- vsyscall.orig/arch/x86/include/asm/pvclock.h
+++ vsyscall/arch/x86/include/asm/pvclock.h
@@ -56,4 +56,32 @@ static inline u64 pvclock_scale_delta(u6
return product;
 }
 
+static __always_inline
+u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
+{
+   u64 delta = __native_read_tsc() - src-tsc_timestamp;
+   return pvclock_scale_delta(delta, src-tsc_to_system_mul,
+  src-tsc_shift);
+}
+
+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;
+}
+
 #endif /* _ASM_X86_PVCLOCK_H */


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html