From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59970) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brn8D-0003Ml-FZ for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:25:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brn88-0001aw-FH for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:25:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brn88-0001ah-9a for qemu-devel@nongnu.org; Wed, 05 Oct 2016 10:25:08 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 D915AC049D5C for ; Wed, 5 Oct 2016 14:25:07 +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> <20161005142010.GE1657@noname.str.redhat.com> From: Paolo Bonzini Message-ID: Date: Wed, 5 Oct 2016 16:25:04 +0200 MIME-Version: 1.0 In-Reply-To: <20161005142010.GE1657@noname.str.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, famz@redhat.com On 05/10/2016 16:20, Kevin Wolf wrote: > 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. Just removing those three lines would be okay. Paolo