[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to IN
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files |
Date: |
Mon, 2 Feb 2015 15:16:17 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 02.02.2015 um 15:12 hat Peter Lieven geschrieben:
> Am 02.02.2015 um 15:04 schrieb Kevin Wolf:
> >Am 02.02.2015 um 14:55 hat Peter Lieven geschrieben:
> >>Am 02.02.2015 um 14:23 schrieb 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.
> >>The default (0) actually means unlimited or undefined. We introduced
> >>that limit of 16MB in bdrv_co_write_zeroes to create only reasonable
> >>sized requests because there is no guarantee that write zeroes is a
> >>fast operation. We should set INT_MAX only if we know that write
> >>zeroes of an arbitrary size is always fast.
> >Well, splitting it up doesn't make it any faster. I think we can assume
> >that drv->bdrv_co_write_zeroes() wants to know the full request size
> >unless the driver has explicitly set bs->bl.max_write_zeroes.
>
> You mean sth like this:
Yes, I think that's what I meant.
Kevin
> diff --git a/block.c b/block.c
> index 61412e9..8272ef9 100644
> --- a/block.c
> +++ b/block.c
> @@ -3192,10 +3192,7 @@ int coroutine_fn
> bdrv_co_copy_on_readv(BlockDriverState *bs,
> BDRV_REQ_COPY_ON_READ);
> }
>
> -/* if no limit is specified in the BlockLimits use a default
> - * of 32768 512-byte sectors (16 MiB) per request.
> - */
> -#define MAX_WRITE_ZEROES_DEFAULT 32768
> +#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768
>
> static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
> int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
> @@ -3206,7 +3203,7 @@ static int coroutine_fn
> bdrv_co_do_write_zeroes(BlockDriverState *bs,
> int ret = 0;
>
> int max_write_zeroes = bs->bl.max_write_zeroes ?
> - bs->bl.max_write_zeroes :
> MAX_WRITE_ZEROES_DEFAULT;
> + bs->bl.max_write_zeroes : INT_MAX;
>
> while (nb_sectors > 0 && !ret) {
> int num = nb_sectors;
> @@ -3242,7 +3239,7 @@ static int coroutine_fn
> bdrv_co_do_write_zeroes(BlockDriverState *bs,
> if (ret == -ENOTSUP) {
> /* Fall back to bounce buffer if write zeroes is unsupported */
> int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length,
> - MAX_WRITE_ZEROES_DEFAULT);
> + MAX_WRITE_ZEROES_BOUNCE_BUFFER);
> num = MIN(num, max_xfer_len);
> iov.iov_len = num * BDRV_SECTOR_SIZE;
> if (iov.iov_base == NULL) {
> @@ -5099,11 +5096,6 @@ static void coroutine_fn bdrv_discard_co_entry(void
> *opaque)
> rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num,
> rwco->nb_sectors);
> }
>
> -/* if no limit is specified in the BlockLimits use a default
> - * of 32768 512-byte sectors (16 MiB) per request.
> - */
> -#define MAX_DISCARD_DEFAULT 32768
> -
> int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
> int nb_sectors)
> {
> @@ -5128,7 +5120,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs,
> int64_t sector_num,
> return 0;
> }
>
> - max_discard = bs->bl.max_discard ? bs->bl.max_discard :
> MAX_DISCARD_DEFAULT;
> + max_discard = bs->bl.max_discard ? bs->bl.max_discard : INT_MAX;
> while (nb_sectors > 0) {
> int ret;
> int num = nb_sectors;
>
>
>
> Peter
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Kevin Wolf, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Peter Lieven, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Kevin Wolf, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Peter Lieven, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Peter Lieven, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Denis V. Lunev, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Kevin Wolf, 2015/02/02
- Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files, Denis V. Lunev, 2015/02/02