From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v3 02/11] hvm/hpet: Only call guest_time_hpet(h) one time per action. Date: Wed, 23 Apr 2014 11:42:54 -0400 Message-ID: <5357DF7E.9000202@terremark.com> References: <1397756585-27091-1-git-send-email-dslutz@verizon.com> <1397756585-27091-3-git-send-email-dslutz@verizon.com> <5357F345020000780000BA05@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5357F345020000780000BA05@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Don Slutz Cc: Keir Fraser , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 4/23/2014 11:07 AM, Jan Beulich wrote: >>>> On 17.04.14 at 19:42, 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 >