[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 02/11] hvm/hpet: Only call guest_time_hpet(h) one time per action.



On 4/23/2014 11:07 AM, Jan Beulich wrote:
On 17.04.14 at 19:42, <dslutz@xxxxxxxxxxx> wrote:
v3:
   Did not add Reviewed-by (Jan Beulich) do to amount of change
   Added passing of guest_time to hpet_read64() and
     hpet_stop_timer().
But that wasn't as a result of the review of v2, was it?

Not on this thread.  I was using gdb to get answers for a later patch.

  Especially for
the hpet_read64() case I fear this is going a little too far, since now
you call the supposedly expensive function even when the value isn't
needed. I suppose you could resolve this by passing a known-invalid
value to the function from hpet_read() (so it knows to call
guest_time_hpet() itself) and use the always-preset variant on from
hpet_write().

I also considered a case statement on the call (since the value is needed later when
hpet_write also uses it).  I do not think that any value is invalid.


Otoh I admit that the only supposedly frequently use operations
ought to be the two ones needing the value, so perhaps the
overhead for the other ones is tolerable. Let's see whether others
have any opinion on this.

I also expect that the frequently used operations are the ones that need it. And it
is the lower risk of add a bug.  Will wait for any other comments.

    -Don Slutz

Apart from that I'm still fine with the patch.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.