From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56558) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zq7DM-0005SO-B6 for qemu-devel@nongnu.org; Sat, 24 Oct 2015 18:23:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zq7DJ-000728-8Z for qemu-devel@nongnu.org; Sat, 24 Oct 2015 18:23:04 -0400 Received: from mail-lf0-x22d.google.com ([2a00:1450:4010:c07::22d]:34314) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zq7DJ-00071D-0u for qemu-devel@nongnu.org; Sat, 24 Oct 2015 18:23:01 -0400 Received: by lfaz124 with SMTP id z124so114957640lfa.1 for ; Sat, 24 Oct 2015 15:23:00 -0700 (PDT) References: <4ae623f2773f233d231531a61f872198cd945afd.1445688993.git.digetx@gmail.com> From: Dmitry Osipenko Message-ID: <562C048E.4020804@gmail.com> Date: Sun, 25 Oct 2015 01:22:06 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v7 1/2] hw/ptimer: Fix issues caused by artificially limited timer timeout List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , QEMU Developers 24.10.2015 22:45, Peter Crosthwaite пишет: > > This looks like a give-up without trying to get the correct value. If > the calculated value (using the normal-path logic below) is sane, you > should just use it. If it comes out bad then you should clamp to 1. > > I am wondering whether this clamping policy (as in original code as > well) is correct at all though. The value of a free-running > short-interval periodic timer (poor mans random number generator) > without any actual interrupt generation will be affected by QEMUs > asynchronous handling of timer events. So if I set limit to 100, then > sample this timer every user keyboard stroke, I should get a uniform > distribution on [0,100]. Instead in am going to get lots of 1s. This Right, that's a good example. What about to scale ptimer period to match adjusted timer_mod interval? > is more broken in the original code (as you state), as I will get > > 100, but I think we have traded broken for slightly less broken. I > think the correct semantic is to completely ignoring rate limitin > except for the scheduling on event callbacks. That is, the timer I'm missing you here. What event callbacks? > interval is not rate limited, instead the timer_mod interval > (next_event -last_event) just has a 10us clamp. > > The means the original code semantic of returning counter = 0 for an > already triggered timer is wrong. It should handle in-the-past > wrap-arounds as wraparounds by triggering the timer and redoing the > math with the new interval values. So instead the logic would be > something like: > > timer_val = -1; > > for(;;) { > > if (!enabled) { > return delta; > } > > timer_val = (next-event - now) * scaling(); > if (timer_val >= 0) { > return timer_val; > } > /* Timer has actually expired but we missed it, reload it and try again */ > ptimer_tick(); > } Why do you think that ptimer_get_count() == 0 in case of the running periodic timer that was expired while QEMU was "on the way" to ptimer code is bad and wrong? From the guest point of view it's okay (no?), do we really need to overengineer that corner case? > > ptimer_reload() then needs to be patched to make sure it always > timer_mod()s in the future, otherwise this loop could iterate a large > number of times. > > This means that when the qemu_timer() actually ticks, a large number > or cycles may have occured, but we can justify that in that callback > event latency (usually interrupts) is undefined anyway and coalescing > of multiples may have happened as part of that. This usually matches > expectations of real guests where interrupt latency is ultimately > undefined. ptimer_tick() is re-arm'ing the qemu_timer() in case of periodic mode. Hope I haven't missed your point here. > > Regards, > Peter > -- Dmitry