From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42851)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1V4owC-0000dk-T5
	for qemu-devel@nongnu.org; Thu, 01 Aug 2013 05:12:54 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1V4ohr-0000Nb-P0
	for qemu-devel@nongnu.org; Thu, 01 Aug 2013 04:58:05 -0400
Received: from mx3-phx2.redhat.com ([209.132.183.24]:54979)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1V4ohr-0000NU-Fc
	for qemu-devel@nongnu.org; Thu, 01 Aug 2013 04:57:59 -0400
Date: Thu, 1 Aug 2013 04:57:52 -0400 (EDT)
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <391717867.8023244.1375347472193.JavaMail.root@redhat.com>
In-Reply-To: <CAJnKYQmL0UnOM=OpfUGiEsZZvs-2EA2B9PutNJONW3osCkFn3w@mail.gmail.com>
References: <1375067768-11342-1-git-send-email-pingfank@linux.vnet.ibm.com>
	<1375067768-11342-4-git-send-email-pingfank@linux.vnet.ibm.com>
	<51F60C02.6020008@redhat.com>
	<CAJnKYQns35_vyr=kyGNEF4__paK1s==38Uc9U+2gvZMKyBbYQw@mail.gmail.com>
	<51F65044.4040706@redhat.com>
	<CAJnKYQmznzjHUFRb_BbDN+3p9KtaS0G6c_RNrgPFetRpk6431w@mail.gmail.com>
	<51F78491.708@redhat.com>
	<CAJnKYQmL0UnOM=OpfUGiEsZZvs-2EA2B9PutNJONW3osCkFn3w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync
 between disable and timer's cb
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: liu ping fan <qemulist@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Jan Kiszka <jan.kiszka@siemens.com>, qemu-devel@nongnu.org, Alex Bligh <alex@alex.org.uk>, Anthony Liguori <anthony@codemonkey.ws>

> > Hmm, do we even need clock->using at this point?  For example:
> >
> >    qemu_clock_enable()
> >    {
> >        clock->enabled = enabled;
> >        ...
> >        if (!enabled) {
> >            /* If another thread is within qemu_run_timers,
> >             * wait for it to finish.
> >             */
> >            qemu_event_wait(&clock->callbacks_done_event);
> >        }
> >    }
> >
> >    qemu_run_timers()
> >    {
> >        qemu_event_reset(&clock->callbacks_done_event);
> >        if (!clock->enabled) {
> >            goto out;
> >        }
> >        ...
> >    out:
> >        qemu_event_set(&eclock->callbacks_done_event);
> >    }
> >
> > In the fast path this only does two atomic operations (an OR for reset,
> > and XCHG for set).
> >
> There is race condition, suppose the following scenario with A/B thread
> A: qemu_event_reset()
> B: qemu_event_reset()
> A: qemu_event_set() ----> B is still in flight when
> qemu_clock_enable() is notified
> B: qemu_event_set()
> 
> I had tried to build something around futex(2) like qemu_event, but failed.

True, qemu_event basically works only when a single thread resets it.  But
there is no race condition here because qemu_run_timers cannot be executed
concurrently by multiple threads (like aio_poll in your bottom half patches).

Paolo