On Tue, 29 May 2012, Stefan Weil wrote: > Am 29.05.2012 15:35, schrieb Stefano Stabellini: > > qemu_rearm_alarm_timer partially duplicates the code in > > qemu_next_alarm_deadline to figure out if it needs to rearm the timer. > > If it calls qemu_next_alarm_deadline, it always rearms the timer even if > > the next deadline is INT64_MAX. > > > > This patch simplifies the behavior of qemu_rearm_alarm_timer and removes > > the duplicated code, always calling qemu_next_alarm_deadline and only > > rearming the timer if the deadline is less than INT64_MAX. > > > > Signed-off-by: Stefano Stabellini<stefano.stabell...@eu.citrix.com> > > > > diff --git a/qemu-timer.c b/qemu-timer.c > > index de98977..81ff824 100644 > > --- a/qemu-timer.c > > +++ b/qemu-timer.c > > @@ -112,14 +112,10 @@ static int64_t qemu_next_alarm_deadline(void) > > > > static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) > > { > > - int64_t nearest_delta_ns; > > - if (!rt_clock->active_timers&& > > - !vm_clock->active_timers&& > > - !host_clock->active_timers) { > > - return; > > + int64_t nearest_delta_ns = qemu_next_alarm_deadline(); > > + if (nearest_delta_ns< INT64_MAX) { > > + t->rearm(t, nearest_delta_ns); > > } > > - nearest_delta_ns = qemu_next_alarm_deadline(); > > - t->rearm(t, nearest_delta_ns); > > } > > > > /* TODO: MIN_TIMER_REARM_NS should be optimized */ > > Reviewed-by: Stefan Weil <s...@weilnetz.de>
thanks > This patch clearly improves the current code and fixes > an abort on Darwin (reported by Andreas Färber) and maybe > other hosts. Therefore I changed the subject and suggest > to consider this patch for QEMU 1.1. > > There remain issues which can be fixed after 1.1: > > nearest_delta_ns also gets negative values (rtdelta < 0, > maybe because the expiration time already expired). > I did not check whether all different timers handle > a negative time gracefully. > > nearest_delta_ns should also be limited to INT32_MAX > seconds, because some timers assign the seconds > to a long (see setitimer) or UINT value. On 32 bit > Linux and on all variants of Windows, long is less > or equal INT32_MAX. If we limit nearest_delta_ns > to 1000000 seconds (or some other limit which allows > ULONG milliseconds), we could further simplify the code > because most timers would no longer have to test the > upper limit. If that's the issue we could limit nearest_delta_ns to LONG_MAX. However I got the feeling that Darwin has an undocumented limit for tv_sec, lower than INT32_MAX.