From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bro5T-00048Y-Ug for qemu-devel@nongnu.org; Wed, 05 Oct 2016 11:26:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bro5N-0002tl-UK for qemu-devel@nongnu.org; Wed, 05 Oct 2016 11:26:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41946) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bro5N-0002td-Nr for qemu-devel@nongnu.org; Wed, 05 Oct 2016 11:26:21 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (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 405A119D397 for ; Wed, 5 Oct 2016 15:26:21 +0000 (UTC) Date: Wed, 5 Oct 2016 17:26:19 +0200 From: Kevin Wolf Message-ID: <20161005152619.GI1657@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> <20161005142010.GE1657@noname.str.redhat.com> 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 16:25 hat Paolo Bonzini geschrieben: > > > 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. Ok, done. Kevin