From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33127)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <qemulist@gmail.com>) id 1V9spH-0005P9-Je
	for qemu-devel@nongnu.org; Thu, 15 Aug 2013 04:22:36 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <qemulist@gmail.com>) id 1V9spG-00064F-LS
	for qemu-devel@nongnu.org; Thu, 15 Aug 2013 04:22:35 -0400
Received: from mail-lb0-x231.google.com ([2a00:1450:4010:c04::231]:39210)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <qemulist@gmail.com>) id 1V9spG-000645-Eh
	for qemu-devel@nongnu.org; Thu, 15 Aug 2013 04:22:34 -0400
Received: by mail-lb0-f177.google.com with SMTP id n6so415297lbi.22
	for <qemu-devel@nongnu.org>; Thu, 15 Aug 2013 01:22:33 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <20130815080124.GC22521@stefanha-thinkpad.redhat.com>
References: <1376311769-29811-1-git-send-email-stefanha@redhat.com>
	<1376311769-29811-3-git-send-email-stefanha@redhat.com>
	<CAJnKYQn5bV-3Pop1TGP51rjB9GRb1F9HB5xrOoDXLYD50v0=hw@mail.gmail.com>
	<20130815080124.GC22521@stefanha-thinkpad.redhat.com>
From: liu ping fan <qemulist@gmail.com>
Date: Thu, 15 Aug 2013 16:22:13 +0800
Message-ID: <CAJnKYQkdWbJynkHTATejB+gOQP1KcdgKO3eqcU1Rs18yofkwkw@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-timer: make qemu_timer_mod_ns()
 and qemu_timer_del() thread-safe
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Alex Bligh <alex@alex.org.uk>, Paolo Bonzini <pbonzini@redhat.com>, Ping Fan Liu <pingfank@linux.vnet.ibm.com>, qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>

On Thu, Aug 15, 2013 at 4:01 PM, Stefan Hajnoczi <stefanha@gmail.com> 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 <stefanha@redhat.com> 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