| Message ID | 1296298848-3080-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Jan 29, 2011 at 12:00:47PM +0100, Paolo Bonzini wrote: > When the QEMU_CLOCK_HOST clock was added, computation of its > deadline was added to qemu_next_deadline, which is correct but > incomplete. > > I noticed this while trying to make sense of the rules whereby > qemu_next_deadline_dyntick is computed, which miss QEMU_CLOCK_HOST > when use_icount is true. Looking at the history showed this to > be just an oversight, as the next patch shows clearly. > > This patch inlines qemu_next_deadline into qemu_next_deadline_dyntick, > and corrects the logic so that only QEMU_CLOCK_VIRTUAL is skipped for > use_icount == true. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Cc: Jan Kiszka <jan.kiszka@siemens.com> > --- > qemu-timer.c | 15 +++++++++++---- > 1 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/qemu-timer.c b/qemu-timer.c > index db1ec49..174fd0c 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -724,11 +724,18 @@ static uint64_t qemu_next_deadline_dyntick(void) > int64_t delta; > int64_t rtdelta; > > - if (use_icount) > + if (!use_icount && active_timers[QEMU_CLOCK_VIRTUAL]) { > + delta = active_timers[QEMU_CLOCK_VIRTUAL]->expire_time - > + qemu_get_clock(vm_clock); > + } else { > delta = INT32_MAX; > - else > - delta = (qemu_next_deadline() + 999) / 1000; > - This computation handled the fact that the value is returned in ns, and we want us. > + } > + if (active_timers[QEMU_CLOCK_HOST]) { > + int64_t hdelta = active_timers[QEMU_CLOCK_HOST]->expire_time - > + qemu_get_clock(host_clock); > + if (hdelta < delta) > + delta = hdelta; > + } And this doesn't appear anymore after your changes. > if (active_timers[QEMU_CLOCK_REALTIME]) { > rtdelta = (active_timers[QEMU_CLOCK_REALTIME]->expire_time - > qemu_get_clock(rt_clock))*1000; Here we multiply because the value is returned in ms and we want us. IMHO we should just use qemu_get_clock_ns() instead, replace MIN_TIMER_REARM_US by MIN_TIMER_REARM_NS, and return a ns value instead. Of course dynticks_rearm_timer() has to be adjusted, but it should not be problematic given it uses ns internally. Otherwise good catch, the issue is real, and the way to fix it looks correct.
diff --git a/qemu-timer.c b/qemu-timer.c index db1ec49..174fd0c 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -724,11 +724,18 @@ static uint64_t qemu_next_deadline_dyntick(void) int64_t delta; int64_t rtdelta; - if (use_icount) + if (!use_icount && active_timers[QEMU_CLOCK_VIRTUAL]) { + delta = active_timers[QEMU_CLOCK_VIRTUAL]->expire_time - + qemu_get_clock(vm_clock); + } else { delta = INT32_MAX; - else - delta = (qemu_next_deadline() + 999) / 1000; - + } + if (active_timers[QEMU_CLOCK_HOST]) { + int64_t hdelta = active_timers[QEMU_CLOCK_HOST]->expire_time - + qemu_get_clock(host_clock); + if (hdelta < delta) + delta = hdelta; + } if (active_timers[QEMU_CLOCK_REALTIME]) { rtdelta = (active_timers[QEMU_CLOCK_REALTIME]->expire_time - qemu_get_clock(rt_clock))*1000;
When the QEMU_CLOCK_HOST clock was added, computation of its deadline was added to qemu_next_deadline, which is correct but incomplete. I noticed this while trying to make sense of the rules whereby qemu_next_deadline_dyntick is computed, which miss QEMU_CLOCK_HOST when use_icount is true. Looking at the history showed this to be just an oversight, as the next patch shows clearly. This patch inlines qemu_next_deadline into qemu_next_deadline_dyntick, and corrects the logic so that only QEMU_CLOCK_VIRTUAL is skipped for use_icount == true. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Cc: Jan Kiszka <jan.kiszka@siemens.com> --- qemu-timer.c | 15 +++++++++++---- 1 files changed, 11 insertions(+), 4 deletions(-)