On 18/12/2007, Anders <[EMAIL PROTECTED]> 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
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)