[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for cry
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB |
Date: |
Fri, 4 Aug 2017 13:32:29 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Fri, Aug 04, 2017 at 07:08:54AM -0500, Eric Blake wrote:
> 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.)
For writes it is even more critical - if we encrypted in place, and the
guest tried to serve a later read from its cache, it'd return cipher
text instead of plain text.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|