From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brmfx-0004zr-K3 for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:56:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brmft-0003oi-HZ for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:56:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35670) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brmft-0003oZ-B2 for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:55:57 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C32B132E88D for ; Wed, 5 Oct 2016 13:55:56 +0000 (UTC) References: <1475511256-20051-1-git-send-email-pbonzini@redhat.com> <1475511256-20051-2-git-send-email-pbonzini@redhat.com> <20161005131318.GF863@stefanha-x1.localdomain> From: Paolo Bonzini Message-ID: Date: Wed, 5 Oct 2016 15:55:52 +0200 MIME-Version: 1.0 In-Reply-To: <20161005131318.GF863@stefanha-x1.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/2] async: add aio_bh_schedule_oneshot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, kwolf@redhat.com, famz@redhat.com On 05/10/2016 15:13, Stefan Hajnoczi wrote: > > qemu_bh_delete is already clearing bh->scheduled at the same time > > as it's setting bh->deleted. Since it's not using any memory > > barriers, there is no synchronization going on for bh->deleted, > > and this makes the bh->deleted checks superfluous in aio_compute_time= out, > > aio_bh_poll and aio_ctx_check. >=20 > Yikes. On one hand this sounds scary but in practice qemu_bh_delete() > isn't called from another thread so the next aio_bh_poll() will indeed > clean it up instead of dispatching a deleted BH. >=20 > Due to the nature of this change I suggest making it in a separate > patch. Separate from what? (Sorry if I'm being dense). >> >> + * aio_bh_schedule_oneshot: Allocate a new bottom half structure that= will run >> + * only once and as soon as possible. >> + * >> + * Bottom halves are lightweight callbacks whose invocation is guaran= teed >> + * to be wait-free, thread-safe and signal-safe. The #QEMUBH structu= re >> + * is opaque and must be allocated prior to its use. >=20 > I'm confused. There is no QEMUBH structure in this function > prototype. Is this comment from an earlier version of this function? No, it's from aio_bh_new. Of course this one is neither wait-free nor signal-safe. Kevin, do you want me to respin? Paolo