[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit
From: |
Eric Blake |
Subject: |
Re: [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit |
Date: |
Tue, 11 May 2021 16:19:45 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to support 64 bit write-zeroes requests. Now update the
> limit variable. It's absolutely safe. The variable is set in some
> drivers, and used in bdrv_co_do_pwrite_zeroes().
>
> Update also max_write_zeroes variable in bdrv_co_do_pwrite_zeroes(), so
> that bdrv_co_do_pwrite_zeroes() is now prepared to 64bit requests. The
> remaining logic including num, offset and bytes variables is already
> supporting 64bit requests.
>
> So the only thing that prevents 64 bit requests is limiting
> max_write_zeroes variable to INT_MAX in bdrv_co_do_pwrite_zeroes().
> We'll drop this limitation after updating all block drivers.
>
> Ah, we also have bdrv_check_request32() in bdrv_co_pwritev_part(). It
> will be modified to do bdrv_check_request() for write-zeroes path.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block_int.h | 7 +++----
> block/io.c | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> +++ b/include/block/block_int.h
> @@ -676,10 +676,9 @@ typedef struct BlockLimits {
> * that is set. May be 0 if bl.request_alignment is good enough */
> uint32_t pdiscard_alignment;
>
> - /* Maximum number of bytes that can zeroized at once (since it is
> - * signed, it must be < 2G, if set). Must be multiple of
> - * pwrite_zeroes_alignment. May be 0 if no inherent 32-bit limit */
> - int32_t max_pwrite_zeroes;
> + /* Maximum number of bytes that can zeroized at once. Must be multiple of
> + * pwrite_zeroes_alignment. May be 0 if no inherent 64-bit limit */
Is the comment still right?
Leaving as 0 is the easiest way for a driver to say "default limit", but
I would feel safer with the default being 2G-align rather than 63-bit
limit. And it is a 63-bit limit, not 64-bit, if the driver opts in to
INT64_MAX.
> + int64_t max_pwrite_zeroes;
>
> /* Optimal alignment for write zeroes requests in bytes. A power
> * of 2 is best but not mandatory. Must be a multiple of
> diff --git a/block/io.c b/block/io.c
> index 55095dd08e..79e600af27 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1836,7 +1836,7 @@ static int coroutine_fn
> bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
> int head = 0;
> int tail = 0;
>
> - int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
> + int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes,
> INT_MAX);
You are correct that for now we have no behavior change; a driver opting
in to a larger limit will still be clamped until we revisit this patch
later to drop the MIN() - but I agree with your approach of keeping
MIN() here until all drivers are audited.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit,
Eric Blake <=