Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files

[Kevin Wolf]

Am 30.01.2015 um 09:42 hat Denis V. Lunev geschrieben:
> fallocate() works fine and could handle properly with arbitrary size
> requests. There is no sense to reduce the amount of space to fallocate.
> The bigger is the size, the better is the performance as the amount of
> journal updates is reduced.
> 
> The patch changes behavior for both generic filesystem and XFS codepaths,
> which are different in handle_aiocb_write_zeroes. The implementation
> of fallocate and xfsctl(XFS_IOC_ZERO_RANGE) for XFS are exactly the same
> thus the change is fine for both ways.
> 
> Signed-off-by: Denis V. Lunev <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Peter Lieven <address@hidden>
> CC: Fam Zheng <address@hidden>
> ---
>  block/raw-posix.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 7b42f37..933c778 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -293,6 +293,20 @@ static void raw_probe_alignment(BlockDriverState *bs, 
> int fd, Error **errp)
>      }
>  }
>  
> +static void raw_probe_max_write_zeroes(BlockDriverState *bs)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    struct stat st;
> +
> +    if (fstat(s->fd, &st) < 0) {
> +        return; /* no problem, keep default value */
> +    }
> +    if (!S_ISREG(st.st_mode) || !s->discard_zeroes) {
> +        return;
> +    }
> +    bs->bl.max_write_zeroes = INT_MAX;
> +}

Peter, do you remember why INT_MAX isn't actually the default? I think
the most reasonable behaviour would be that a limitation is only used if
a block driver requests it, and otherwise unlimited is assumed.

We can take this patch to raw-posix, it is certainly not wrong. But any
format driver or filter will still, in most cases needlessly, apply
MAX_WRITE_ZEROES_DEFAULT, i.e. a 16 MB maximum, so I think we should
consider making a change to the default.

Kevin