Hollis Blanchard wrote:
On Mon, 2008-07-07 at 15:56 +0200, ehrhardt@xxxxxxxxxxxxxxxxxx wrote:
From: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>
The current implementation of kvmtrace uses always a 64 bit cycle variable,
but get_cycles() which is used to fill it is "unsigned long" which might be 32
bit.
This reduces the accuracy e.g. on embedded powerpc since we would have a 64bit
value but get_cycle() only returns the low 32 bit.
To solve that this patch introduces kvm_arch_trace_cycles() which allows us
to make this calculation architecture aware. That way every architecture can
insert whatever fits best for their "kvmtrace cycle counter".
Signed-off-by: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>
"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 if
we could rename the data structure too, but I'd settle for only properly
naming the new architecture function hook.
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
continued it to the kvm_arch function.
I'll change the naming - timestamp would be fine, but only if it is
really time and not a cycle counter.
I experimented with the tv_nsec portion and the classic current_time and
accuracy there was just far to low compared to the time base register
data (5 to 20 instructions until tv.nssec portion changed).
I checked the getnstimeofday function you mentioned and compared
accuracy again and yes that is exactly what I would like to use:
here a ittle example
263527418457 (+ 36696) ns=0x1c43286a
263527740677 (+ 322220) ns=0x1c4a880c (+ 75FA2 = 483234)
263527779757 (+ 39080) ns=0x1c4b6cae (+ E4A2 = 58530)
While the unit is not the same (explains the difference in the same line
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
over the file.
For the example I had above
cycle = 322220/39080 = 8.25
nsec = 483234/58530 = 8.25
That leads me to the point where I completely agree with Hollis that the
naming should be timestamp as well as that the function should rely on
getnstimeofday() instead of get_cycles() and that way would remove the
need kvm_arch_ function.
So the question that is left before changing that is, if the original
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
agree *hope* on using getnstimeofday() - after an ack from all sides I
would revise my patch series and submit that changes alltogether.
--
Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
