From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: Re: [PATCH 2/5] kvmtrace: make cycle calculation architecture aware Date: Wed, 09 Jul 2008 11:17:19 +0200 Message-ID: <4874821F.4060509@linux.vnet.ibm.com> References: <1215439013-11480-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215439013-11480-3-git-send-email-ehrhardt@linux.vnet.ibm.com> <1215448660.11175.15.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, avi@qumranet.com, kvm-ppc@vger.kernel.org, eric.e.liu@intel.com To: Hollis Blanchard , cotte@de.ibm.com, xiantao.zhang@intel.com Return-path: Received: from mtagate7.de.ibm.com ([195.212.29.156]:6001 "EHLO mtagate7.de.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751892AbYGIJSX (ORCPT ); Wed, 9 Jul 2008 05:18:23 -0400 In-Reply-To: <1215448660.11175.15.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: Hollis Blanchard wrote: > On Mon, 2008-07-07 at 15:56 +0200, ehrhardt@linux.vnet.ibm.com wrote: > =20 >> From: Christian Ehrhardt >> >> The current implementation of kvmtrace uses always a 64 bit cycle va= riable, >> but get_cycles() which is used to fill it is "unsigned long" which m= ight be 32 >> bit. >> This reduces the accuracy e.g. on embedded powerpc since we would ha= ve a 64bit >> value but get_cycle() only returns the low 32 bit. >> To solve that this patch introduces kvm_arch_trace_cycles() which al= lows us >> to make this calculation architecture aware. That way every architec= ture can >> insert whatever fits best for their "kvmtrace cycle counter". >> >> Signed-off-by: Christian Ehrhardt >> =20 > > "cycles" is a very poor name, because that's not really what we're > talking about at all. (Also, that function name made me wonder what a > "trace cycle" is. :) > > I would strongly prefer using "timestamp" instead. It would be nice i= f > we could rename the data structure too, but I'd settle for only prope= rly > naming the new architecture function hook. > =20 > In fact, if we want to be rigorous about it, it should really be > something like "nanoseconds" instead, so that userspace wouldn't need= to > perform awkward conversions of "cycles" or "timebase ticks" to real > time. It looks like getnstimeofday() would do the trick, and that way= we > wouldn't need an arch-specific hook at all I agree that the naming is poor. I also wondered about it and just=20 continued it to the kvm_arch function. I'll change the naming - timestamp would be fine, but only if it is=20 really time and not a cycle counter. I experimented with the tv_nsec portion and the classic current_time an= d=20 accuracy there was just far to low compared to the time base register=20 data (5 to 20 instructions until tv.nssec portion changed). I checked the getnstimeofday function you mentioned and compared=20 accuracy again and yes that is exactly what I would like to use: here a ittle example 263527418457 (+ 36696) ns=3D0x1c43286a 263527740677 (+ 322220) ns=3D0x1c4a880c (+ 75FA2 =3D 483234) 263527779757 (+ 39080) ns=3D0x1c4b6cae (+ E4A2 =3D 58530) While the unit is not the same (explains the difference in the same lin= e=20 e.g. 39080 vs. 58530) the accuracy is the same. this can be seen due to the fact that it changes by the same ratio all=20 over the file. =46or the example I had above cycle =3D 322220/39080 =3D 8.25 nsec =3D 483234/58530 =3D 8.25 That leads me to the point where I completely agree with Hollis that th= e=20 naming should be timestamp as well as that the function should rely on=20 getnstimeofday() instead of get_cycles() and that way would remove the=20 need kvm_arch_ function. So the question that is left before changing that is, if the original=20 author had something special in mind chosing cycles here. I added Eric on CC for that. I wait with my resubmission of the patch series until all architectures= =20 agree *hope* on using getnstimeofday() - after an ack from all sides I=20 would revise my patch series and submit that changes alltogether. --=20 Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization