diff mbox

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

Message ID 1422366699-17473-8-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Jan. 27, 2015, 1:51 p.m. UTC
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.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Peter Lieven <pl@kamp.de>
CC: Fam Zheng <famz@redhat.com>
---
 block/raw-posix.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Max Reitz Jan. 27, 2015, 6:05 p.m. UTC | #1
On 2015-01-27 at 08:51, Denis V. Lunev wrote:
> fallocate() works fine and could handle properly with arbitrary size
> requests.

Maybe "could properly handle arbitrary size requests" (or 
"...arbitrarily sized 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.

True for fallocate(). But is it true for xfs_write_zeroes(), too? I 
guess so, but I don't know.

If it does, the patch looks good to me.

Max

> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Peter Lieven <pl@kamp.de>
> CC: Fam Zheng <famz@redhat.com>
> ---
>   block/raw-posix.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
Denis V. Lunev Jan. 27, 2015, 6:11 p.m. UTC | #2
On 27/01/15 21:05, Max Reitz wrote:
> On 2015-01-27 at 08:51, Denis V. Lunev wrote:
>> fallocate() works fine and could handle properly with arbitrary size
>> requests.
>
> Maybe "could properly handle arbitrary size requests" (or 
> "...arbitrarily sized 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.
>
> True for fallocate(). But is it true for xfs_write_zeroes(), too? I 
> guess so, but I don't know.
>
> If it does, the patch looks good to me.
>
> Max
>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Peter Lieven <pl@kamp.de>
>> CC: Fam Zheng <famz@redhat.com>
>> ---
>>   block/raw-posix.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
thank you very much for a review. I will proceed with these findings,
they look quite reasonable.
Denis V. Lunev Jan. 28, 2015, 6:39 a.m. UTC | #3
On 27/01/15 21:05, Max Reitz wrote:
> On 2015-01-27 at 08:51, Denis V. Lunev wrote:
>> fallocate() works fine and could handle properly with arbitrary size
>> requests.
>
> Maybe "could properly handle arbitrary size requests" (or 
> "...arbitrarily sized 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.
>
> True for fallocate(). But is it true for xfs_write_zeroes(), too? I 
> guess so, but I don't know.
>
> If it does, the patch looks good to me.
>
> Max
>

checked.

xfs_ioc_space (ioctl handler) calls exactly the same xfs_zero_file_space as
performed by xfs_file_fallocate on fallocate path.

I will reflect this in the description

Regards,
     Den
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index fa05239..e0b35c9 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -292,6 +292,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;
+}
+
 static void raw_parse_flags(int bdrv_flags, int *open_flags)
 {
     assert(open_flags != NULL);
@@ -598,6 +612,7 @@  static int raw_reopen_prepare(BDRVReopenState *state,
     /* Fail already reopen_prepare() if we can't get a working O_DIRECT
      * alignment with the new fd. */
     if (raw_s->fd != -1) {
+        raw_probe_max_write_zeroes(state->bs);
         raw_probe_alignment(state->bs, raw_s->fd, &local_err);
         if (local_err) {
             qemu_close(raw_s->fd);
@@ -651,6 +666,8 @@  static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 
     raw_probe_alignment(bs, s->fd, errp);
     bs->bl.opt_mem_alignment = s->buf_align;
+
+    raw_probe_max_write_zeroes(bs);
 }
 
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)