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

[Avi Kivity <avi@qumranet.com>]

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