| Message ID | 1381222058-16701-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Paolo, On 8 Oct 2013, at 09:47, Paolo Bonzini wrote: > > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -393,11 +393,40 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) > } > } > > +/* modify the current timer so that it will be fired when current_time > + >= expire_time or the current deadline, whichever comes earlier. > + The corresponding callback will be called. */ > +void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time) > +{ > + QEMUTimerList *timer_list = ts->timer_list; > + bool rearm; > + > + qemu_mutex_lock(&timer_list->active_timers_lock); > + if (ts->expire_time == -1 || ts->expire_time > expire_time) { So "if we want to alter it" ... > + if (ts->expire_time != -1) { > + timer_del_locked(timer_list, ts); > + } What's this bit for? Surely you've calculated whether you are shortening the expiry time (above), so all you need do now is modify it. Why delete it? timer_mod_ns doesn't make this check? Otherwise looks OK. > + rearm = timer_mod_ns_locked(timer_list, ts, expire_time); > + } else { > + rearm = false; > + } > + qemu_mutex_unlock(&timer_list->active_timers_lock); > + > + if (rearm) { > + timerlist_rearm(timer_list); > + } > +} > + > void timer_mod(QEMUTimer *ts, int64_t expire_time) > { > timer_mod_ns(ts, expire_time * ts->scale); > } > > +void timer_mod_anticipate(QEMUTimer *ts, int64_t expire_time) > +{ > + timer_mod_anticipate_ns(ts, expire_time * ts->scale); > +} > + > bool timer_pending(QEMUTimer *ts) > { > return ts->expire_time >= 0; > -- > 1.8.3.1 > > > >
Il 08/10/2013 11:15, Alex Bligh ha scritto: > So "if we want to alter it" ... > >> > + if (ts->expire_time != -1) { >> > + timer_del_locked(timer_list, ts); >> > + } > What's this bit for? Surely you've calculated whether you are > shortening the expiry time (above), so all you need do now is > modify it. Why delete it? timer_mod_ns doesn't make this > check? timer_mod_ns_locked does not remove the timer from the list: qemu_mutex_lock(&timer_list->active_timers_lock); timer_del_locked(timer_list, ts); rearm = timer_mod_ns_locked(timer_list, ts, expire_time); qemu_mutex_unlock(&timer_list->active_timers_lock); This is doing the same. The check in the "if" is not strictly necessary, but it saves a useless visit of the list. It could be added to timer_mod_ns as well. Paolo
On 8 Oct 2013, at 10:25, Paolo Bonzini wrote: > Il 08/10/2013 11:15, Alex Bligh ha scritto: >> So "if we want to alter it" ... >> >>>> + if (ts->expire_time != -1) { >>>> + timer_del_locked(timer_list, ts); >>>> + } >> What's this bit for? Surely you've calculated whether you are >> shortening the expiry time (above), so all you need do now is >> modify it. Why delete it? timer_mod_ns doesn't make this >> check? > > timer_mod_ns_locked does not remove the timer from the list: > > qemu_mutex_lock(&timer_list->active_timers_lock); > timer_del_locked(timer_list, ts); > rearm = timer_mod_ns_locked(timer_list, ts, expire_time); > qemu_mutex_unlock(&timer_list->active_timers_lock); > > This is doing the same. The check in the "if" is not strictly > necessary, but it saves a useless visit of the list. It could be added > to timer_mod_ns as well. Quite right. I'd missed the timer_del somehow.
diff --git a/include/qemu/timer.h b/include/qemu/timer.h index b58903b..f215b0b 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -539,6 +539,19 @@ void timer_del(QEMUTimer *ts); void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); /** + * timer_mod_anticipate_ns: + * @ts: the timer + * @expire_time: the expiry time in nanoseconds + * + * Modify a timer to expire at @expire_time or the current time, + * whichever comes earlier. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. + */ +void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time); + +/** * timer_mod: * @ts: the timer * @expire_time: the expire time in the units associated with the timer @@ -552,6 +565,19 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); void timer_mod(QEMUTimer *ts, int64_t expire_timer); /** + * timer_mod_anticipate: + * @ts: the timer + * @expire_time: the expiry time in nanoseconds + * + * Modify a timer to expire at @expire_time or the current time, whichever + * comes earlier, taking into account the scale associated with the timer. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. + */ +void timer_mod_anticipate(QEMUTimer *ts, int64_t expire_time); + +/** * timer_pending: * @ts: the timer * diff --git a/qemu-timer.c b/qemu-timer.c index 95fc6eb..202e9a2 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -393,11 +393,40 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) } } +/* modify the current timer so that it will be fired when current_time + >= expire_time or the current deadline, whichever comes earlier. + The corresponding callback will be called. */ +void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time) +{ + QEMUTimerList *timer_list = ts->timer_list; + bool rearm; + + qemu_mutex_lock(&timer_list->active_timers_lock); + if (ts->expire_time == -1 || ts->expire_time > expire_time) { + if (ts->expire_time != -1) { + timer_del_locked(timer_list, ts); + } + rearm = timer_mod_ns_locked(timer_list, ts, expire_time); + } else { + rearm = false; + } + qemu_mutex_unlock(&timer_list->active_timers_lock); + + if (rearm) { + timerlist_rearm(timer_list); + } +} + void timer_mod(QEMUTimer *ts, int64_t expire_time) { timer_mod_ns(ts, expire_time * ts->scale); } +void timer_mod_anticipate(QEMUTimer *ts, int64_t expire_time) +{ + timer_mod_anticipate_ns(ts, expire_time * ts->scale); +} + bool timer_pending(QEMUTimer *ts) { return ts->expire_time >= 0;
These let a user anticipate the deadline of a timer, atomically with other sites that call the function. This helps avoiding complicated lock hierarchies. It is useful whenever the timer does work based on the current value of the clock (rather than doing something periodically on every tick). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- include/qemu/timer.h | 26 ++++++++++++++++++++++++++ qemu-timer.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+)