Re: [Qemu-devel] [PATCH v2 06/20] block: make bdrv_co_do_write_zeroes stricter in producing aligned requests

[Peter Lieven]

On 19.11.2013 18:07, Paolo Bonzini wrote:
> Right now, bdrv_co_do_write_zeroes will only try to align the
> beginning of the request.  However, it is simpler for many
> formats to expect the block layer to separate both the head *and*
> the tail.  This makes sure that the format's bdrv_co_write_zeroes
> function will be called with aligned sector_num and nb_sectors for
> the bulk of the request.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>   block.c | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/block.c b/block.c
> index 4897649..567d669 100644
> --- a/block.c
> +++ b/block.c
> @@ -2761,14 +2761,19 @@ static int coroutine_fn 
> bdrv_co_do_write_zeroes(BlockDriverState *bs,
>       while (nb_sectors > 0 && !ret) {
>           int num = nb_sectors;
>   
> -        /* align request */
> -        if (bs->bl.write_zeroes_alignment &&
> -            num >= bs->bl.write_zeroes_alignment &&
> -            sector_num % bs->bl.write_zeroes_alignment) {
> -            if (num > bs->bl.write_zeroes_alignment) {
> +        /* Align request.  Block drivers can expect the "bulk" of the request
> +         * to be aligned.
> +         */
> +        if (bs->bl.write_zeroes_alignment
> +            && num > bs->bl.write_zeroes_alignment) {
> +            if (sector_num % bs->bl.write_zeroes_alignment != 0) {
> +                /* Make a small request up to the first aligned sector.  */
>                   num = bs->bl.write_zeroes_alignment;
> +                num -= sector_num % bs->bl.write_zeroes_alignment;
> +            } else if (num >= bs->bl.write_zeroes_alignment) {
> +                /* Shorten the request to the last aligned sector.  */
> +                num -= (sector_num + num) % bs->bl.write_zeroes_alignment;
>               }
> -            num -= sector_num % bs->bl.write_zeroes_alignment;
>           }
>   
>           /* limit request size */
> @@ -2785,24 +2790,27 @@ static int coroutine_fn 
> bdrv_co_do_write_zeroes(BlockDriverState *bs,
>           if (ret == -ENOTSUP) {
>               /* Fall back to bounce buffer if write zeroes is unsupported */
>               iov.iov_len = num * BDRV_SECTOR_SIZE;
>               if (iov.iov_base == NULL) {
> -                /* allocate bounce buffer only once and ensure that it
> -                 * is big enough for this and all future requests.
> -                 */
> -                size_t bufsize = num <= nb_sectors ? num : max_write_zeroes;
> -                iov.iov_base = qemu_blockalign(bs, bufsize * BDRV_SECTOR_SIZE);
> -                memset(iov.iov_base, 0, bufsize * BDRV_SECTOR_SIZE);
> +                iov.iov_base = qemu_blockalign(bs, num * BDRV_SECTOR_SIZE);
> +                memset(iov.iov_base, 0, num * BDRV_SECTOR_SIZE);
>               }
>               qemu_iovec_init_external(&qiov, &iov, 1);
>   
>               ret = drv->bdrv_co_writev(bs, sector_num, num, &qiov);
> +            if (num <= max_write_zeroes) {
This frees the bounce buffer ever and ever. num can never be greater than
max_write_zeroes. So you need a num < max_write_zeroes here.

Peter
> +                /* Allocate bounce buffer only once if it is
> +                 * big enough for this and all future requests.
> +                 */
> +                qemu_vfree(iov.iov_base);
> +                iov.iov_base = NULL;
> +            }
>           }
>   
>           sector_num += num;
>           nb_sectors -= num;
>       }
>   
>       qemu_vfree(iov.iov_base);
>       return ret;
>   }
>   


--

Mit freundlichen Grüßen

Peter Lieven

...........................................................

  KAMP Netzwerkdienste GmbH
  Vestische Str. 89-91 | 46117 Oberhausen
  Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
  address@hidden | http://www.kamp.de

  Geschäftsführer: Heiner Lante | Michael Lante
  Amtsgericht Duisburg | HRB Nr. 12154
  USt-Id-Nr.: DE 120607556

...........................................................