Re: [PATCH 2/9] kvmtrace: replace get_cycles with getnstimeofday

Avi Kivity
2008-07-10T15:15:08Z

ehrhardt@linux.vnet.ibm.com wrote:
> From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
>
> The current kvmtrace code uses get_cycles() while the interpretation would be
> easier using using nanoseconds. Getnstimeofday should give the same accuracy
> as get_cycles on all architectures but at a better unit (e.g. comparable
> between two hosts with different frequencies.
>
> The implementation of kvmtrace uses a 64 bit cycle variable while get_cycles
> only provided a unsigned long which is != 64 bit on 32 bit architectures.
> Since this patch introduced getnstimeofday we can addtionally merge the
> tv_sec portion of the struct timespec returned to use the full 64 bit
> of the trace interface and therefor avoid some wraparounds. This merge
> is done always if the two values of a struct timespec fit into the u64.
>
> An alternative might be ktime_t instead of u64 and using as Sheng Yang
> suggested and ktime_get(), I had no time to test&compare that yet.
> Any comments or requirements from others what to prefer here ?
>
> Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
> ---
>
> [diffstat]
> kvm_trace.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> [diff]
>
> diff --git a/virt/kvm/kvm_trace.c b/virt/kvm/kvm_trace.c
> --- a/virt/kvm/kvm_trace.c
> +++ b/virt/kvm/kvm_trace.c
> @@ -56,6 +56,7 @@
> struct kvm_vcpu *vcpu;
> int i, size;
> u32 extra;
> + struct timespec tv;
>
> if (unlikely(kt->trace_state != KVM_TRACE_STATE_RUNNING))
> return;
> @@ -73,7 +74,13 @@
> | TRACE_REC_NUM_DATA_ARGS(extra);
>
> if (p->cycle_in) {
> - rec.u.cycle.cycle_u64 = get_cycles();
> + getnstimeofday(&tv);
> + /* compress both struct tv values into u64 on 32 bit archs */
> + if (sizeof(tv) == sizeof(u64))
> + rec.u.cycle.cycle_u64 =
> + (((u64)tv.tv_sec) << 32) | tv.tv_nsec;
> + else
> + rec.u.cycle.cycle_u64 = tv.tv_nsec;
> (*args, u32);
>

Surely, different results on 32-bit and 64-bit aren't what's wanted. I
guess ktime_get() is better.

--
error compiling committee.c: too many arguments to function