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 <vsement...@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