[Qemu-devel] [PATCH v3 0/1] block: enforce minimal 4096 alignment in qemu_blockalign

[Qemu-devel] [PATCH v3 0/1] block: enforce minimal 4096 alignment in qemu_blockalign

[Denis V. Lunev]

The following sequence
    int fd = open(argv[1], O_RDWR | O_CREAT | O_DIRECT, 0644);
    for (i = 0; i < 100000; i++)
            write(fd, buf, 4096);
iperforms 5% better if buf is aligned to 4096 bytes rather then to
512 bytes.

I have used the following program to test
#define _GNU_SOURCE

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/types.h>
#include <malloc.h>
#include <string.h>

int main(int argc, char *argv[])
{
    int fd = open(argv[1], O_RDWR | O_CREAT | O_DIRECT, 0644);
    void *buf;
    int i = 0, align = atoi(argv[2]);

    do {
        buf = memalign(align, 4096);
        if (align >= 4096)
            break;
        if ((unsigned long)buf & 4095)
            break;
        i++;
    } while (1);
    printf("%d %p\n", i, buf);

    memset(buf, 0x11, 4096);

    for (i = 0; i < 100000; i++) {
        lseek(fd, SEEK_CUR, 4096);
        write(fd, buf, 4096);
    }

    close(fd);
    return 0;
}
for in in `seq 1 30` ; do a.out aa ; done

The file was placed into 8 GB partition on HDD below to avoid speed
change due to different offset on disk. Results are reliable:
- 189 vs 180 seconds on Linux 3.16

The following setups have been tested:
1) ext4 with block size equals to 1024 over 512/512 physical/logical
   sector size SSD disk
2) ext4 with block size equals to 4096 over 512/512 physical/logical
   sector size SSD disk
3) ext4 with block size equals to 4096 over 512/4096 physical/logical
   sector size rotational disk (WDC WD20EZRX)
4) xfs with block size equals to 4096 over 512/512 physical/logical
   sector size SSD disk

The difference is quite reliable and the same 5%.
  qemu-io -n -c 'write -P 0xaa 0 1G' 1.img
for image in qcow2 format is 1% faster.

Changes from v2:
- opt_mem_alignment is split to opt_mem_alignment for bounce buffering
  and min_mem_alignment to check buffers coming from guest.

Changes from v1:
- enforces 4096 alignment in qemu_(try_)blockalign, avoid touching of
  bdrv_qiov_is_aligned path not to enforce additional bounce buffering
  as suggested by Paolo
- reduces 10% to 5% in patch description to better fit 180 vs 189
  difference

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>

[Qemu-devel] [PATCH 1/1] block: align bounce buffers to page

[Denis V. Lunev]

The following sequence
    int fd = open(argv[1], O_RDWR | O_CREAT | O_DIRECT, 0644);
    for (i = 0; i < 100000; i++)
            write(fd, buf, 4096);
performs 5% better if buf is aligned to 4096 bytes.

The difference is quite reliable.

On the other hand we do not want at the moment to enforce bounce
buffering if guest request is aligned to 512 bytes. This patch
forces page alignment when we are really forced to perform bounce
buffering.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c                   | 19 +++++++++++++++++--
 block/raw-posix.c         |  3 ++-
 include/block/block.h     |  2 ++
 include/block/block_int.h |  3 +++
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index d45e4dd..d17c5ef 100644
--- a/block.c
+++ b/block.c
@@ -232,6 +232,16 @@ size_t bdrv_opt_mem_align(BlockDriverState *bs)
     return bs->bl.opt_mem_alignment;
 }
 
+size_t bdrv_min_mem_align(BlockDriverState *bs)
+{
+    if (!bs || !bs->drv) {
+        /* 4k should be on the safe side */
+        return 4096;
+    }
+
+    return bs->bl.min_mem_alignment;
+}
+
 /* check if the path starts with "<protocol>:" */
 int path_has_protocol(const char *path)
 {
@@ -542,8 +552,10 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.opt_transfer_length = bs->file->bl.opt_transfer_length;
         bs->bl.max_transfer_length = bs->file->bl.max_transfer_length;
         bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
+        bs->bl.min_mem_alignment = bs->file->bl.min_mem_alignment;
     } else {
-        bs->bl.opt_mem_alignment = 512;
+        bs->bl.opt_mem_alignment = 4096;
+        bs->bl.min_mem_alignment = 512;
     }
 
     if (bs->backing_hd) {
@@ -561,6 +573,9 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.opt_mem_alignment =
             MAX(bs->bl.opt_mem_alignment,
                 bs->backing_hd->bl.opt_mem_alignment);
+        bs->bl.min_mem_alignment =
+            MAX(bs->bl.min_mem_alignment,
+                bs->backing_hd->bl.min_mem_alignment);
     }
 
     /* Then let the driver override it */
@@ -5331,7 +5346,7 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
 bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
 {
     int i;
-    size_t alignment = bdrv_opt_mem_align(bs);
+    size_t alignment = bdrv_min_mem_align(bs);
 
     for (i = 0; i < qiov->niov; i++) {
         if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 933c778..3ad2e5e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -667,7 +667,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
     BDRVRawState *s = bs->opaque;
 
     raw_probe_alignment(bs, s->fd, errp);
-    bs->bl.opt_mem_alignment = s->buf_align;
+    bs->bl.min_mem_alignment = s->buf_align;
+    bs->bl.opt_mem_alignment = MAX(sysconf(_SC_PAGESIZE), s->buf_align);
 
     raw_probe_max_write_zeroes(bs);
 }
diff --git a/include/block/block.h b/include/block/block.h
index 3082d2b..b9b24b5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -424,6 +424,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
 
 /* Returns the alignment in bytes that is required so that no bounce buffer
  * is required throughout the stack */
+size_t bdrv_min_mem_align(BlockDriverState *bs);
+/* Returns optimal alignment in bytes for bounce buffer */
 size_t bdrv_opt_mem_align(BlockDriverState *bs);
 void bdrv_set_guest_block_size(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e264be9..7dccac4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -296,6 +296,9 @@ typedef struct BlockLimits {
     int max_transfer_length;
 
     /* memory alignment so that no bounce buffer is needed */
+    size_t min_mem_alignment;
+
+    /* memory alignment for effective transfers when bounce buffer is needed */
     size_t opt_mem_alignment;
 } BlockLimits;
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/1] block: align bounce buffers to page

[Paolo Bonzini]

On 02/02/2015 17:44, Denis V. Lunev wrote:
> +++ b/block/raw-posix.c
> @@ -667,7 +667,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
>      BDRVRawState *s = bs->opaque;
>  
>      raw_probe_alignment(bs, s->fd, errp);
> -    bs->bl.opt_mem_alignment = s->buf_align;
> +    bs->bl.min_mem_alignment = s->buf_align;
> +    bs->bl.opt_mem_alignment = MAX(sysconf(_SC_PAGESIZE), s->buf_align);

This does not exist on Windows.  You have to use getpagesize(), for
which there is a wrapper in util/oslib-win32.c.

Paolo

>      raw_probe_max_write_zeroes(bs);

Re: [Qemu-devel] [PATCH 1/1] block: align bounce buffers to page

[Denis V. Lunev]

On 02/02/15 19:47, Paolo Bonzini wrote:
>
> On 02/02/2015 17:44, Denis V. Lunev wrote:
>> +++ b/block/raw-posix.c
>> @@ -667,7 +667,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
>>       BDRVRawState *s = bs->opaque;
>>   
>>       raw_probe_alignment(bs, s->fd, errp);
>> -    bs->bl.opt_mem_alignment = s->buf_align;
>> +    bs->bl.min_mem_alignment = s->buf_align;
>> +    bs->bl.opt_mem_alignment = MAX(sysconf(_SC_PAGESIZE), s->buf_align);
> This does not exist on Windows.  You have to use getpagesize(), for
> which there is a wrapper in util/oslib-win32.c.
>
> Paolo
>
>>       raw_probe_max_write_zeroes(bs);
cool suggestion, will do that. Thank you