Re: [Qemu-devel] [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB

[Eric Blake]

On 08/04/2017 05:51 AM, Daniel P. Berrange wrote:
> Using 16KB bounce buffers creates a significant performance
> penalty for I/O to encrypted volumes on storage with high
> I/O latency (rotating rust & network drives), because it
> triggers lots of fairly small I/O operations.
> 
> On tests with rotating rust, and cache=none|directsync,
> write speed increased from 2MiB/s to 32MiB/s, on a par
> with that achieved by the in-kernel luks driver.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  block/crypto.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 58ef6f2f52..207941db9a 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -379,7 +379,7 @@ static void block_crypto_close(BlockDriverState *bs)
>  }
>  
>  
> -#define BLOCK_CRYPTO_MAX_SECTORS 32
> +#define BLOCK_CRYPTO_MAX_SECTORS 2048
>  
>  static coroutine_fn int
>  block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
> @@ -396,9 +396,8 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
> sector_num,
>  
>      qemu_iovec_init(&hd_qiov, qiov->niov);
>  
> -    /* Bounce buffer so we have a linear mem region for
> -     * entire sector. XXX optimize so we avoid bounce
> -     * buffer in case that qiov->niov == 1
> +    /* Bounce buffer because we're not permitted to touch
> +     * contents of qiov - it points to guest memory.

The comment updates are accurate (and in line with your other patch for
improving documentation of the callback semantics), but slightly
unrelated to the fix at hand. However, I have no problem keeping it in
the patch.

(To make sure I understand the importance of the bounce buffer: On
reads, we can't store into the buffer until we have decrypted, so that
the guest can't transiently spy on the encrypted form; on writes, we
must read from the buffer at most once before encrypting, so that the
guest can't change the buffer under our feet while we are encrypting.)

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature