Re: [Qemu-devel] [PATCH 2/2] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe

[liu ping fan]

On Thu, Aug 15, 2013 at 4:01 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Thu, Aug 15, 2013 at 08:05:11AM +0800, liu ping fan wrote:
>> On Mon, Aug 12, 2013 at 8:49 PM, Stefan Hajnoczi <address@hidden> wrote:
>> > @@ -376,13 +411,16 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
>> >
>> >      current_time = qemu_clock_get_ns(timer_list->clock->type);
>> >      for(;;) {
>> > +        qemu_mutex_lock(&timer_list->active_timers_lock);
>> >          ts = timer_list->active_timers;
>> >          if (!timer_expired_ns(ts, current_time)) {
>> > +            qemu_mutex_unlock(&timer_list->active_timers_lock);
>> >              break;
>> >          }
>> >          /* remove timer from the list before calling the callback */
>> >          timer_list->active_timers = ts->next;
>> >          ts->next = NULL;
>> > +        qemu_mutex_unlock(&timer_list->active_timers_lock);
>> >
>> Could we do better than this? lock/unlock around ts->cb always cause extra 
>> cost?
>> Beside this, others seems good.
>
> ts->cb() can do anything.  We need to drop the mutex to prevent
> recursive locking.
>
> There is no cheap way to clone the list before the loop (so that we
> don't need to hold any lock while iterating), and the list may change
> when ts->cb() is called.
>
> Did you have a specific improvement in mind?
>
How about new_list for vcpu to add timer, an before walking, splice
the new_list to timer_list?
> Stefan