From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49431) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Upa33-0006qL-2K for qemu-devel@nongnu.org; Thu, 20 Jun 2013 04:16:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Upa31-0004jj-QD for qemu-devel@nongnu.org; Thu, 20 Jun 2013 04:16:52 -0400 Received: from mail-qe0-f53.google.com ([209.85.128.53]:51162) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Upa31-0004jf-M3 for qemu-devel@nongnu.org; Thu, 20 Jun 2013 04:16:51 -0400 Received: by mail-qe0-f53.google.com with SMTP id 1so3782223qee.40 for ; Thu, 20 Jun 2013 01:16:51 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51C2BA6B.2050706@redhat.com> Date: Thu, 20 Jun 2013 10:16:43 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1371675569-6516-1-git-send-email-pingfank@linux.vnet.ibm.com> <1371675569-6516-3-git-send-email-pingfank@linux.vnet.ibm.com> <20130620073924.GA14255@stefanha-thinkpad.redhat.com> In-Reply-To: <20130620073924.GA14255@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Liu Ping Fan , Liu Ping Fan , Anthony Liguori , qemu-devel@nongnu.org Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto: > qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch. > > It seems that calling them from a thread is a little risky because there > is no guarantee that the BH is no longer invoked after a thread calls > these functions. > > I think that's worth a comment or do you want them to take the lock so > they become safe? Taking the lock wouldn't help. The invoking loop of aio_bh_poll runs lockless. I think a comment is better. qemu_bh_cancel is inherently not thread-safe, there's not much you can do about it. qemu_bh_delete is safe as long as you wait for the bottom half to stop before deleting the containing object. Once we have RCU, deletion of QOM objects will be RCU-protected. Hence, a simple way could be to put the first part of aio_bh_poll() within rcu_read_lock/unlock. > The other thing I'm unclear on is the ->idle assignment followed > immediately by a ->scheduled assignment. Without memory barriers > aio_bh_poll() isn't guaranteed to get an ordered view of these updates: > it may see an idle BH as a regular scheduled BH because ->idle is still > 0. Right. You need to order ->idle writes before ->scheduled writes, and add memory barriers, or alternatively use two bits in ->scheduled so that you can assign both atomically. Paolo