From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56427) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1w5Q-0002kp-Pi for qemu-devel@nongnu.org; Tue, 01 Jul 2014 07:19:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X1w5K-0007hb-Fi for qemu-devel@nongnu.org; Tue, 01 Jul 2014 07:18:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1w5K-0007hT-6V for qemu-devel@nongnu.org; Tue, 01 Jul 2014 07:18:50 -0400 Date: Tue, 1 Jul 2014 13:18:43 +0200 From: Kevin Wolf Message-ID: <20140701111843.GE4587@noname.str.redhat.com> References: <1404201119-6646-1-git-send-email-ming.lei@canonical.com> <1404201119-6646-2-git-send-email-ming.lei@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1404201119-6646-2-git-send-email-ming.lei@canonical.com> Subject: Re: [Qemu-devel] [PATCH v2 1/3] block: block: introduce bdrv_io_plug() and bdrv_io_unplug() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei Cc: Peter Maydell , Fam Zheng , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini Am 01.07.2014 um 09:51 hat Ming Lei geschrieben: > This patch introduces these two APIs so that following > patches can support queuing I/O requests and submitting them > at batch for improving I/O performance. > > Reviewed-by: Paolo Bonzini > Signed-off-by: Ming Lei > --- > block.c | 21 +++++++++++++++++++++ > include/block/block.h | 3 +++ > include/block/block_int.h | 4 ++++ > 3 files changed, 28 insertions(+) > > diff --git a/block.c b/block.c > index 217f523..fea9e43 100644 > --- a/block.c > +++ b/block.c > @@ -1910,6 +1910,7 @@ void bdrv_drain_all(void) > bool bs_busy; > > aio_context_acquire(aio_context); > + bdrv_io_unplug(bs); > bdrv_start_throttled_reqs(bs); > bs_busy = bdrv_requests_pending(bs); > bs_busy |= aio_poll(aio_context, bs_busy); This means that bdrv_io_plug/unplug() are not paired as I would have expected. I find the name not very descriptive anyway (I probably wouldn't have guessed what it does from its name if it weren't in this series), so maybe we should consider renaming it? Perhaps something like bdrv_req_batch_start() and bdrv_req_batch_submit(), but I'm open for different suggestions. > @@ -5774,3 +5775,23 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate) > > return false; > } > + > +void bdrv_io_plug(BlockDriverState *bs) > +{ > + BlockDriver *drv = bs->drv; > + if (drv && drv->bdrv_io_plug) { > + drv->bdrv_io_plug(bs); > + } else if (bs->file) { > + bdrv_io_plug(bs->file); > + } > +} Does this bs->file forwarding work for more than the raw driver? For example, if drv is an image format driver that needs to read some metadata from the image before it can submit the payload, does this still do what you were intending? Kevin