From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brn3T-0007dX-3R for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:20:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brn3M-0007CQ-Ex for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:20:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brn3M-0007C8-8w for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:20:12 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 D4FE7A0A51 for ; Wed, 5 Oct 2016 14:20:11 +0000 (UTC) Date: Wed, 5 Oct 2016 16:20:10 +0200 From: Kevin Wolf Message-ID: <20161005142010.GE1657@noname.str.redhat.com> References: <1475511256-20051-1-git-send-email-pbonzini@redhat.com> <1475511256-20051-2-git-send-email-pbonzini@redhat.com> <20161005131318.GF863@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: Paolo Bonzini Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, famz@redhat.com Am 05.10.2016 um 15:55 hat Paolo Bonzini geschrieben: > 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_timeout, > > > aio_bh_poll and aio_ctx_check. > > > > 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. > > > > 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 guaranteed > >> + * to be wait-free, thread-safe and signal-safe. The #QEMUBH structure > >> + * is opaque and must be allocated prior to its use. > > > > 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? If the comment is wrong, either post a v2 of this patch or just reply with a new version of the comment and I'll squash it in. Your choice, I don't mind either way. Kevin