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

[Qemu-devel] [PATCH v4 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 v3:
- portable way to calculate system page size used
- 512/4096 values are replaced with proper macros/values

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>

[Qemu-devel] [PATCH 1/2] block, raw-posix: replace 512/4096 constants with proper macros/values

[Denis V. Lunev]

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block.c           | 10 +++++-----
 block/raw-posix.c | 16 ++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index d45e4dd..e98d651 100644
--- a/block.c
+++ b/block.c
@@ -225,8 +225,8 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs,
 size_t bdrv_opt_mem_align(BlockDriverState *bs)
 {
     if (!bs || !bs->drv) {
-        /* 4k should be on the safe side */
-        return 4096;
+        /* page size should be on the safe side */
+        return getpagesize();
     }
 
     return bs->bl.opt_mem_alignment;
@@ -543,7 +543,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.max_transfer_length = bs->file->bl.max_transfer_length;
         bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
     } else {
-        bs->bl.opt_mem_alignment = 512;
+        bs->bl.opt_mem_alignment = BDRV_SECTOR_SIZE;
     }
 
     if (bs->backing_hd) {
@@ -965,8 +965,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     }
 
     bs->open_flags = flags;
-    bs->guest_block_size = 512;
-    bs->request_alignment = 512;
+    bs->guest_block_size = BDRV_SECTOR_SIZE;
+    bs->request_alignment = BDRV_SECTOR_SIZE;
     bs->zero_beyond_eof = true;
     open_flags = bdrv_open_flags(bs, flags);
     bs->read_only = !(open_flags & BDRV_O_RDWR);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 853ffa6..9848f83 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -122,7 +122,6 @@
    reopen it to see if the disk has been changed */
 #define FD_OPEN_TIMEOUT (1000000000)
 
-#define MAX_BLOCKSIZE	4096
 
 typedef struct BDRVRawState {
     int fd;
@@ -222,6 +221,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
     BDRVRawState *s = bs->opaque;
     char *buf;
     unsigned int sector_size;
+    size_t page_size = getpagesize();
 
     /* For /dev/sg devices the alignment is not really used.
        With buffered I/O, we don't have any restrictions. */
@@ -264,9 +264,9 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
     /* If we could not get the sizes so far, we can only guess them */
     if (!s->buf_align) {
         size_t align;
-        buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE);
-        for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
-            if (pread(fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) {
+        buf = qemu_memalign(page_size, 2 * page_size);
+        for (align = BDRV_SECTOR_SIZE; align <= page_size; align <<= 1) {
+            if (pread(fd, buf + align, page_size, 0) >= 0) {
                 s->buf_align = align;
                 break;
             }
@@ -276,8 +276,8 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 
     if (!bs->request_alignment) {
         size_t align;
-        buf = qemu_memalign(s->buf_align, MAX_BLOCKSIZE);
-        for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
+        buf = qemu_memalign(s->buf_align, page_size);
+        for (align = BDRV_SECTOR_SIZE; align <= page_size; align <<= 1) {
             if (pread(fd, buf, align, 0) >= 0) {
                 bs->request_alignment = align;
                 break;
@@ -1269,7 +1269,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
         struct dkwedge_info dkw;
 
         if (ioctl(fd, DIOCGWEDGEINFO, &dkw) != -1) {
-            return dkw.dkw_size * 512;
+            return dkw.dkw_size * BDRV_SECTOR_SIZE;
         } else {
             struct disklabel dl;
 
@@ -1401,7 +1401,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     if (fstat(s->fd, &st) < 0) {
         return -errno;
     }
-    return (int64_t)st.st_blocks * 512;
+    return (int64_t)st.st_blocks * BDRV_SECTOR_SIZE;
 }
 
 static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/2] block, raw-posix: replace 512/4096 constants with proper macros/values

[Kevin Wolf]

Am 06.02.2015 um 18:37 hat Denis V. Lunev geschrieben:
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c           | 10 +++++-----
>  block/raw-posix.c | 16 ++++++++--------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/block.c b/block.c
> index d45e4dd..e98d651 100644
> --- a/block.c
> +++ b/block.c
> @@ -225,8 +225,8 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs,
>  size_t bdrv_opt_mem_align(BlockDriverState *bs)
>  {
>      if (!bs || !bs->drv) {
> -        /* 4k should be on the safe side */
> -        return 4096;
> +        /* page size should be on the safe side */
> +        return getpagesize();

Can we make this MAX(4096, getpagesize())? The 4k aren't chosen because
of buffer alignment in RAM, but because of a disk sector size of 4k is
the highest common value.

>      }
>  
>      return bs->bl.opt_mem_alignment;
> @@ -543,7 +543,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>          bs->bl.max_transfer_length = bs->file->bl.max_transfer_length;
>          bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
>      } else {
> -        bs->bl.opt_mem_alignment = 512;
> +        bs->bl.opt_mem_alignment = BDRV_SECTOR_SIZE;

I wouldn't make this conversion. The value happens to be the same, but
BDRV_SECTOR_SIZE is just the arbitrary unit that the block layer uses
internally for things like sector_num/nb_sectors.

The 512 in this place, however, is the minimum alignment that hardware
could require, and logically independent of BDRV_SECTOR_SIZE.

Similar considerations apply to the other conversions of 512 in this
patch. They would all be better left as literal 512 (unless you want to
introduce new constants for their specific purpose).

>      }
>  
>      if (bs->backing_hd) {
> @@ -965,8 +965,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>      }
>  
>      bs->open_flags = flags;
> -    bs->guest_block_size = 512;
> -    bs->request_alignment = 512;
> +    bs->guest_block_size = BDRV_SECTOR_SIZE;
> +    bs->request_alignment = BDRV_SECTOR_SIZE;
>      bs->zero_beyond_eof = true;
>      open_flags = bdrv_open_flags(bs, flags);
>      bs->read_only = !(open_flags & BDRV_O_RDWR);
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 853ffa6..9848f83 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -122,7 +122,6 @@
>     reopen it to see if the disk has been changed */
>  #define FD_OPEN_TIMEOUT (1000000000)
>  
> -#define MAX_BLOCKSIZE	4096
>  
>  typedef struct BDRVRawState {
>      int fd;
> @@ -222,6 +221,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>      BDRVRawState *s = bs->opaque;
>      char *buf;
>      unsigned int sector_size;
> +    size_t page_size = getpagesize();
>  
>      /* For /dev/sg devices the alignment is not really used.
>         With buffered I/O, we don't have any restrictions. */
> @@ -264,9 +264,9 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>      /* If we could not get the sizes so far, we can only guess them */
>      if (!s->buf_align) {
>          size_t align;
> -        buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE);
> -        for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
> -            if (pread(fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) {
> +        buf = qemu_memalign(page_size, 2 * page_size);
> +        for (align = BDRV_SECTOR_SIZE; align <= page_size; align <<= 1) {
> +            if (pread(fd, buf + align, page_size, 0) >= 0) {
>                  s->buf_align = align;
>                  break;
>              }

Here, I'd prefer MAX(getpagesize(), MAX_BLOCKSIZE) as well.

Kevin

Re: [Qemu-devel] [PATCH 1/2] block, raw-posix: replace 512/4096 constants with proper macros/values

[Denis V. Lunev]

On 16/02/15 13:32, Kevin Wolf wrote:
> Am 06.02.2015 um 18:37 hat Denis V. Lunev geschrieben:
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   block.c           | 10 +++++-----
>>   block/raw-posix.c | 16 ++++++++--------
>>   2 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index d45e4dd..e98d651 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -225,8 +225,8 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs,
>>   size_t bdrv_opt_mem_align(BlockDriverState *bs)
>>   {
>>       if (!bs || !bs->drv) {
>> -        /* 4k should be on the safe side */
>> -        return 4096;
>> +        /* page size should be on the safe side */
>> +        return getpagesize();
> Can we make this MAX(4096, getpagesize())? The 4k aren't chosen because
> of buffer alignment in RAM, but because of a disk sector size of 4k is
> the highest common value.
>
>>       }
>>   
>>       return bs->bl.opt_mem_alignment;
>> @@ -543,7 +543,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>>           bs->bl.max_transfer_length = bs->file->bl.max_transfer_length;
>>           bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
>>       } else {
>> -        bs->bl.opt_mem_alignment = 512;
>> +        bs->bl.opt_mem_alignment = BDRV_SECTOR_SIZE;
> I wouldn't make this conversion. The value happens to be the same, but
> BDRV_SECTOR_SIZE is just the arbitrary unit that the block layer uses
> internally for things like sector_num/nb_sectors.
>
> The 512 in this place, however, is the minimum alignment that hardware
> could require, and logically independent of BDRV_SECTOR_SIZE.
>
> Similar considerations apply to the other conversions of 512 in this
> patch. They would all be better left as literal 512 (unless you want to
> introduce new constants for their specific purpose).
>
>>       }
>>   
>>       if (bs->backing_hd) {
>> @@ -965,8 +965,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>>       }
>>   
>>       bs->open_flags = flags;
>> -    bs->guest_block_size = 512;
>> -    bs->request_alignment = 512;
>> +    bs->guest_block_size = BDRV_SECTOR_SIZE;
>> +    bs->request_alignment = BDRV_SECTOR_SIZE;
>>       bs->zero_beyond_eof = true;
>>       open_flags = bdrv_open_flags(bs, flags);
>>       bs->read_only = !(open_flags & BDRV_O_RDWR);
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 853ffa6..9848f83 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -122,7 +122,6 @@
>>      reopen it to see if the disk has been changed */
>>   #define FD_OPEN_TIMEOUT (1000000000)
>>   
>> -#define MAX_BLOCKSIZE	4096
>>   
>>   typedef struct BDRVRawState {
>>       int fd;
>> @@ -222,6 +221,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>>       BDRVRawState *s = bs->opaque;
>>       char *buf;
>>       unsigned int sector_size;
>> +    size_t page_size = getpagesize();
>>   
>>       /* For /dev/sg devices the alignment is not really used.
>>          With buffered I/O, we don't have any restrictions. */
>> @@ -264,9 +264,9 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>>       /* If we could not get the sizes so far, we can only guess them */
>>       if (!s->buf_align) {
>>           size_t align;
>> -        buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE);
>> -        for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
>> -            if (pread(fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) {
>> +        buf = qemu_memalign(page_size, 2 * page_size);
>> +        for (align = BDRV_SECTOR_SIZE; align <= page_size; align <<= 1) {
>> +            if (pread(fd, buf + align, page_size, 0) >= 0) {
>>                   s->buf_align = align;
>>                   break;
>>               }
> Here, I'd prefer MAX(getpagesize(), MAX_BLOCKSIZE) as well.
>
> Kevin
this makes sense. OK

[Qemu-devel] [PATCH 2/2] 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.

The patch introduces new concept: minimal memory alignment for bounce
buffers. Original so called "optimal" value is actually minimal required
value for aligment. Optimal should be set to page size by default.
There is no driver which should change this default at the moment.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 20 ++++++++++++++++++--
 block/raw-posix.c         |  2 +-
 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 e98d651..569a2d8 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) {
+        /* page size should be on the safe side */
+        return getpagesize();
+    }
+
+    return bs->bl.min_mem_alignment;
+}
+
 /* check if the path starts with "<protocol>:" */
 int path_has_protocol(const char *path)
 {
@@ -541,9 +551,11 @@ 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.min_mem_alignment = bs->file->bl.min_mem_alignment;
         bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
     } else {
-        bs->bl.opt_mem_alignment = BDRV_SECTOR_SIZE;
+        bs->bl.min_mem_alignment = BDRV_SECTOR_SIZE;
+        bs->bl.opt_mem_alignment = getpagesize();
     }
 
     if (bs->backing_hd) {
@@ -558,6 +570,9 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
         bs->bl.max_transfer_length =
             MIN_NON_ZERO(bs->bl.max_transfer_length,
                          bs->backing_hd->bl.max_transfer_length);
+        bs->bl.min_mem_alignment =
+            MAX(bs->bl.min_mem_alignment,
+                bs->backing_hd->bl.min_mem_alignment);
         bs->bl.opt_mem_alignment =
             MAX(bs->bl.opt_mem_alignment,
                 bs->backing_hd->bl.opt_mem_alignment);
@@ -1044,6 +1059,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     }
 
     assert(bdrv_opt_mem_align(bs) != 0);
+    assert(bdrv_min_mem_align(bs) != 0);
     assert((bs->request_alignment != 0) || bs->sg);
     return 0;
 
@@ -5331,7 +5347,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 9848f83..1205e9d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -650,7 +650,7 @@ 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;
 }
 
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
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..98b183c 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 bounce buffer */
     size_t opt_mem_alignment;
 } BlockLimits;
 
-- 
1.9.1

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

[Kevin Wolf]

Am 06.02.2015 um 18:37 hat Denis V. Lunev geschrieben:
> 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.
> 
> The patch introduces new concept: minimal memory alignment for bounce
> buffers. Original so called "optimal" value is actually minimal required
> value for aligment. Optimal should be set to page size by default.
> There is no driver which should change this default at the moment.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>

I don't think the subject lines describes the patch correctly. IIUC,
patch 1 introduces a hard enforcement of page size alignment, and this
patch only loosens it again.

Perhaps the order of patches should be changed so that min_mem_alignment
is introduced first (without changes in the behaviour), and only then
opt_mem_alignment is increased.

Kevin

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

[Denis V. Lunev]

On 16/02/15 13:59, Kevin Wolf wrote:
> Am 06.02.2015 um 18:37 hat Denis V. Lunev geschrieben:
>> 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.
>>
>> The patch introduces new concept: minimal memory alignment for bounce
>> buffers. Original so called "optimal" value is actually minimal required
>> value for aligment. Optimal should be set to page size by default.
>> There is no driver which should change this default at the moment.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
> I don't think the subject lines describes the patch correctly. IIUC,
> patch 1 introduces a hard enforcement of page size alignment, and this
> patch only loosens it again.
>
> Perhaps the order of patches should be changed so that min_mem_alignment
> is introduced first (without changes in the behaviour), and only then
> opt_mem_alignment is increased.
>
> Kevin
ok :)

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

[Denis V. Lunev]

On 06/02/15 20:37, Denis V. Lunev wrote:
> 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 v3:
> - portable way to calculate system page size used
> - 512/4096 values are replaced with proper macros/values
>
> 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>
>
ping