From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KCzCf-0005uF-Ti for qemu-devel@nongnu.org; Sun, 29 Jun 2008 11:52:37 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KCzCf-0005tG-DO for qemu-devel@nongnu.org; Sun, 29 Jun 2008 11:52:37 -0400 Received: from [199.232.76.173] (port=54595 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KCzCf-0005tD-Aq for qemu-devel@nongnu.org; Sun, 29 Jun 2008 11:52:37 -0400 Received: from il.qumranet.com ([212.179.150.194]:59687) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KCzCe-00076Y-NX for qemu-devel@nongnu.org; Sun, 29 Jun 2008 11:52:37 -0400 Received: from gleb-debian.qumranet.com (unknown [172.16.15.143]) by il.qumranet.com (Postfix) with ESMTP id 88BAF250310 for ; Sun, 29 Jun 2008 18:52:35 +0300 (IDT) Date: Sun, 29 Jun 2008 18:52:35 +0300 From: Gleb Natapov Subject: Re: [Qemu-devel] Re: [PATCH 2/3] Fix time drift problem under high load when PIT is in use. Message-ID: <20080629155235.GC12972@minantech.com> References: <20080629140120.5626.1590.stgit@gleb-debian.qumranet.com.qumranet.com> <20080629140225.5626.16864.stgit@gleb-debian.qumranet.com.qumranet.com> <48679EEB.7020500@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <48679EEB.7020500@web.de> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Sun, Jun 29, 2008 at 04:40:43PM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > Count the number of interrupts that was lost due to interrupt coalescing > > and re-inject them back when possible. This fixes time drift problem when > > pit is used as a time source. > > > > Signed-off-by: Gleb Natapov > > --- > > > > hw/i8254.c | 20 +++++++++++++++++++- > > 1 files changed, 19 insertions(+), 1 deletions(-) > > > > diff --git a/hw/i8254.c b/hw/i8254.c > > index 4813b03..c4f0f46 100644 > > --- a/hw/i8254.c > > +++ b/hw/i8254.c > > @@ -61,6 +61,8 @@ static PITState pit_state; > > > > static void pit_irq_timer_update(PITChannelState *s, int64_t current_time); > > > > +static uint32_t pit_irq_coalesced; > > + > > static int pit_get_count(PITChannelState *s) > > { > > uint64_t d; > > @@ -369,12 +371,28 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t current_time) > > return; > > expire_time = pit_get_next_transition_time(s, current_time); > > irq_level = pit_get_out1(s, current_time); > > - qemu_set_irq(s->irq, irq_level); > > + if(irq_level) { > > + if(!qemu_irq_raise(s->irq)) > > + pit_irq_coalesced++; > > + } else { > > + qemu_irq_lower(s->irq); > > + if(pit_irq_coalesced > 0) { > > + if(qemu_irq_raise(s->irq)) > > + pit_irq_coalesced--; > > + qemu_irq_lower(s->irq); > > + } > > + } > > That's graspable for my poor brain: reinject one coalesced IRQ right > after the falling edge of an in-time delivery... This works because I speed up timer that calls this function. I can create another timer just for injection of lost interrupts instead of doing it here. > > > + > > #ifdef DEBUG_PIT > > printf("irq_level=%d next_delay=%f\n", > > irq_level, > > (double)(expire_time - current_time) / ticks_per_sec); > > #endif > > + if(pit_irq_coalesced && expire_time != -1) { > > + uint32_t div = ((pit_irq_coalesced >> 10) & 0x7f) + 2; > > + expire_time -= ((expire_time - current_time) / div); > > + } > > + > > ... but could you comment on this algorithm? I think I got what it does: > splitting up the next regular period in short intervals. But there are a > bit too many magic numbers involved. Where do they come from? What > happens if pit_irq_coalesced becomes large (or: how large can it become > without risking an IRQ storm on the guest)? A few comments would help, I > guess. > The numbers are come from empirical data :) The algorithm divide next time interval by value that depends on number of coalesced interrupts. The divider value can be any number between 2 and 129. The formula is: div = (pit_irq_coalesced / A) % B + 2 I chose A and B so that bit operation could be used instead of arithmetic. -- Gleb.