From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39170) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2GEL-0004Ts-Fs for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:49:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2GEC-0001FQ-41 for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:49:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5777) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2GEB-0001FD-Rt for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:49:20 -0400 Message-ID: <53B3C784.2030504@redhat.com> Date: Wed, 02 Jul 2014 10:49:08 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1404201119-6646-1-git-send-email-ming.lei@canonical.com> <1404201119-6646-2-git-send-email-ming.lei@canonical.com> <20140701111843.GE4587@noname.str.redhat.com> <20140701152130.GK4587@noname.str.redhat.com> <53B2E82D.8020000@redhat.com> <53B3C059.3060601@redhat.com> <20140702083817.GA5996@noname.str.redhat.com> In-Reply-To: <20140702083817.GA5996@noname.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Kevin Wolf Cc: Peter Maydell , Fam Zheng , "Michael S. Tsirkin" , Ming Lei , qemu-devel , Stefan Hajnoczi Il 02/07/2014 10:38, Kevin Wolf ha scritto: > > On top of this, there _could_ be reasons for formats to implement > > plug/unplug themselves. They could coalesce metadata reads or > > copy-on-write operations, for example. This however is independent > > from the default behavior, which IMO is "plugging should propagate > > along bs->file" (and possibly bs->backing_hd too, but not now > > because that opens more cans of worms). > > Yes, enabling it for bs->backing_hd was my alternative option for the > case that we keep bs->file. What can of worms do you see there? From the > reasoning above, I can't see why bs->backing_hd should be any different > from bs->file. The only one I thought of, is someone changing the backing file chain while the backing file is plugged. IIRC bs->file doesn't change when you do bdrv_reopen. Though changing the backing file chain probably would call bdrv_drain_all too. > Another point I was thinking about (not for this series, but in a > follow-up) is what to do with bdrv_aio_multiwrite(). We could either > leave the two different interfaces there, just that multiwrite should > probably call plug/unplug before it sends its requests (this would apply > the improvements to virtio-blk without dataplane; could be done easily > even now), or we try to implement request merging with the plug/unplug > interface and convert the callers. Not sure how we would do the latter, > though, because we don't have a list of requests any more, except in the > lowest layer that actually queues. > > Or should we have a default plug/unplug implementation in block.c that > queues any reads and writes, and on unplug merges requests, then plugs > bs->file and sends the requests to the driver? Longer term this is a good point. Plug/unplug certainly provides many optimization opportunities, but as long as we expose the right API to the device model we need not care about it now. In fact, there are plenty of other places where we can plug/unplug: mirroring, I/O throttling, image formats could also use aio to submit requests to a fragmented bs->file together and plug/unplug around the loop. Benchmarking is needed of course to see if this actually improves performance (unlikely except in the last case) or at least reduces CPU usage. Paolo