From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/9] kvmtrace: replace get_cycles with getnstimeofday Date: Thu, 10 Jul 2008 18:15:05 +0300 Message-ID: <48762779.5070703@qumranet.com> References: <1215687256-18155-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215687256-18155-3-git-send-email-ehrhardt@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, hollisb@us.ibm.com, kvm-ppc@vger.kernel.org To: ehrhardt@linux.vnet.ibm.com Return-path: Received: from il.qumranet.com ([212.179.150.194]:45960 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752233AbYGJPPI (ORCPT ); Thu, 10 Jul 2008 11:15:08 -0400 In-Reply-To: <1215687256-18155-3-git-send-email-ehrhardt@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: ehrhardt@linux.vnet.ibm.com wrote: > From: Christian Ehrhardt > > 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 > --- > > [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