qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 4/5] block: Perform copy-on-read in loop


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v2 4/5] block: Perform copy-on-read in loop
Date: Thu, 5 Oct 2017 11:36:52 -0400
User-agent: Mutt/1.9.0 (2017-09-02)

On Tue, Oct 03, 2017 at 08:43:46PM -0500, Eric Blake wrote:
> Improve our braindead copy-on-read implementation.  Pre-patch,
> we have multiple issues:
> - we create a bounce buffer and perform a write for the entire
> request, even if the active image already has 99% of the
> clusters occupied, and really only needs to copy-on-read the
> remaining 1% of the clusters
> - our bounce buffer was as large as the read request, and can
> needlessly exhaust our memory by using double the memory of
> the request size (the original request plus our bounce buffer),
> rather than a capped maximum overhead beyond the original
> - if a driver has a max_transfer limit, we are bypassing the
> normal code in bdrv_aligned_preadv() that fragments to that
> limit, and instead attempt to read the entire buffer from the
> driver in one go, which some drivers may assert on
> - a client can request a large request of nearly 2G such that
> rounding the request out to cluster boundaries results in a
> byte count larger than 2G.  While this cannot exceed 32 bits,
> it DOES have some follow-on problems:
> -- the call to bdrv_driver_pread() can assert for exceeding
> BDRV_REQUEST_MAX_BYTES, if the driver is old and lacks
> .bdrv_co_preadv
> -- if the buffer is all zeroes, the subsequent call to
> bdrv_co_do_pwrite_zeroes is a no-op due to a negative size,
> which means we did not actually copy on read
> 
> Fix all of these issues by breaking up the action into a loop,
> where each iteration is capped to sane limits.  Also, querying
> the allocation status allows us to optimize: when data is
> already present in the active layer, we don't need to bounce.
> 
> Note that the code has a telling comment that copy-on-read
> should probably be a filter driver rather than a bolt-in hack
> in io.c; but that remains a task for another day.
> 
> CC: address@hidden
> Signed-off-by: Eric Blake <address@hidden>
> 
> ---
> v2: avoid uninit ret on 0-length op [patchew, Kevin]
> ---
>  block/io.c | 120 
> +++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 82 insertions(+), 38 deletions(-)

Reviewed-by: Stefan Hajnoczi <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]