Re: [Qemu-devel] [PATCH 2/8] timers: add timer_mod_anticipate and timer_mod_anticipate_ns

[Alex Bligh]

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
> 
> 
> 
> 

-- 
Alex Bligh