Am 02.02.2015 um 15:20 hat Peter Lieven geschrieben:
Am 02.02.2015 um 15:16 schrieb Kevin Wolf:
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.
I can't find the original discussion why we added this limit. It was actually
the default
before we introduced BlockLimits. And, it was also the default in the
unsupported path
of write zeroes which created big memory allocations. This might be the reason
why
we introduced a limit.
Commit c31cb707 added the limit to bdrv_co_do_write_zeroes(). Before, we
used a bounce buffer of unbounded size.
Anyway, it seems that none of us can think of a reason not to apply the
patch to block.c. Let's just do it, and if it does break something,
we'll figure it out. Can you send it as a proper patch?
Denis, if we apply that patch, would you be okay with dropping 7/7 from
this series, or would still something be missing?
Kevin