From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56658) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aeKIi-0001vx-Vi for qemu-devel@nongnu.org; Fri, 11 Mar 2016 05:28:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aeKIe-0000Ir-Rg for qemu-devel@nongnu.org; Fri, 11 Mar 2016 05:28:08 -0500 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:33500) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aeKIe-0000Il-Ka for qemu-devel@nongnu.org; Fri, 11 Mar 2016 05:28:04 -0500 Received: by mail-wm0-x244.google.com with SMTP id n186so1581524wmn.0 for ; Fri, 11 Mar 2016 02:28:04 -0800 (PST) Sender: Paolo Bonzini References: <1455470231-5223-1-git-send-email-pbonzini@redhat.com> <1455470231-5223-6-git-send-email-pbonzini@redhat.com> <56E01544.6060305@de.ibm.com> <56E01D3F.1060204@redhat.com> <56E03333.5020601@de.ibm.com> <56E04C9B.7070801@redhat.com> <20160310015154.GD23632@ad.usersys.redhat.com> <56E13849.3060409@de.ibm.com> <56E14101.4030405@de.ibm.com> From: Paolo Bonzini Message-ID: <56E29DB1.6000200@redhat.com> Date: Fri, 11 Mar 2016 11:28:01 +0100 MIME-Version: 1.0 In-Reply-To: <56E14101.4030405@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 5/8] virtio-blk: fix "disabled data plane" mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger , Fam Zheng Cc: Bo Tu , qemu-devel@nongnu.org On 10/03/2016 10:40, Christian Borntraeger wrote: > On 03/10/2016 10:03 AM, Christian Borntraeger wrote: >> On 03/10/2016 02:51 AM, Fam Zheng wrote: >> [...] >>> The aio_poll() inside "blk_set_aio_context(s->conf->conf.blk, s->ctx)" looks >>> suspicious: >>> >>> main thread iothread >>> ---------------------------------------------------------------------------- >>> virtio_blk_handle_output() >>> virtio_blk_data_plane_start() >>> vblk->dataplane_started = true; >>> blk_set_aio_context() >>> bdrv_set_aio_context() >>> bdrv_drain() >>> aio_poll() >>> >>> virtio_blk_handle_output() >>> /* s->dataplane_started is true */ >>> !!! -> virtio_blk_handle_request() >>> event_notifier_set(ioeventfd) >>> aio_poll() >>> virtio_blk_handle_request() >>> >>> Christian, could you try the followed patch? The aio_poll above is replaced >>> with a "limited aio_poll" that doesn't disptach ioeventfd. >>> >>> (Note: perhaps moving "vblk->dataplane_started = true;" after >>> blk_set_aio_context() also *works around* this.) >>> >>> --- >>> >>> diff --git a/block.c b/block.c >>> index ba24b8e..e37e8f7 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -4093,7 +4093,9 @@ void bdrv_attach_aio_context(BlockDriverState *bs, >>> >>> void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context) >>> { >>> - bdrv_drain(bs); /* ensure there are no in-flight requests */ >>> + /* ensure there are no in-flight requests */ >>> + bdrv_drained_begin(bs); >>> + bdrv_drained_end(bs); >>> >>> bdrv_detach_aio_context(bs); >>> >> >> That seems to do the trick. > > Or not. Crashed again :-( I would put bdrv_drained_end just before aio_context_release. But secondarily, I'm thinking of making the logic simpler to understand in two ways: 1) adding a mutex around virtio_blk_data_plane_start/stop. 2) moving event_notifier_set(virtio_queue_get_host_notifier(s->vq)); virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true); to a bottom half (created with aio_bh_new in s->ctx). The bottom half takes the mutex, checks again "if (vblk->dataplane_started)" and if it's true starts the processing. Thanks, Paolo