Re: [Qemu-devel] qemu vl.c

["andrzej zaborowski" <balrogg@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

On 18/12/2007, Anders <mail@flac.kalibalik.dk> wrote:
>
> >>>>>   Redundant timer rearm optimisation by Anders Melchiorsen.
>
> > I'm merging qemu-cvs into the kvm repository now, and with this commit
> > in, kvm will hang after about a minute.  Attaching to it with gdb or
> > strace will cause it to resume, so this is very likely a missing signal
> > problem.
>
> That is clearly not good.
>
> I still cannot see anything wrong with my patch. As fewer signals are
> delivered now, it is possible that this change highlights some old race.

Yes, I don't see anything wrong in this change either but I see how
the scenario given by Paul was already buggy before. Paul confirmed on
IRC that this is also what he meant.

I suspect it was buggy since "unix" and "rtc" stopped being the only
available clock sources.

> It is certainly also possible that my patch is all wrong, even if I am not
> able to see it.
>
>
> The next few days I will be travelling, but when I get back, I was
> planning to address Pauls concern by adding
>
>   qemu_rearm_alarm_timer(alarm_timer);

That would be also my first guess at fixing this, but I have no good
test-case (obviously I wouldn't have committed the change if I had
such a test-case).

>
> to the bottom of qemu_mod_timer().
>
> If you feel like it, you can try that out. I am predicting that I will
> have a hard time reproducing your hang, as I have only been testing with
> kvm, and obviously not seen this problem yet.
>
> (What I really plan to add is a rearm only if the new timer is added at
> the head of the list, but the above should do for a test)

Attached is a small patch to do exactly this. The rearming in
main_loop_wait() can also be made conditional but I don't think there
would be a real gain.

Testing with KVM will be appreciated.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: qemu-timers.patch --]
[-- Type: text/x-patch; name=qemu-timers.patch, Size: 402 bytes --]

diff --git a/vl.c b/vl.c
index 70950f8..7552fde 100644
--- a/vl.c
+++ b/vl.c
@@ -1041,6 +1041,10 @@ void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
     ts->expire_time = expire_time;
     ts->next = *pt;
     *pt = ts;
+
+    /* rearm if the necessary */
+    if (pt == &active_timers[ts->clock->type])
+        qemu_rearm_alarm_timer(alarm_timer);
 }
 
 int qemu_timer_pending(QEMUTimer *ts)