Skip to contentSkip to conversation

Open disk images with O_DIRECT

PATCH Superseded Laurent Vivier 9 participants 31 messages Dec 3, 2007 – Dec 5, 2007

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Laurent Vivier

author
#0 lore raw

v2 cover Open disk images with O_DIRECT

These patches allow to open file using O_DIRECT and bypass the host I/O cache.

The v2 is a new version including comments from Anthony Liguori ("directio" has been renamed "cache"), from Balazs Attila-Mihaly (for Win32 support, not tested) and Samuel Thibault (for the generic function qemu_memalign()".

[PATCH 1/2] Add "cache" parameter to "-drive"

Using "cache=off" with "-drive" will open the disk image file using "O_DIRECT".

[PATCH 2/2] Direct IDE I/O

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

Laurent

author
#1 lore raw

PATCH v2 1/2 Add "cache" parameter to "-drive"

This patch adds a new parameter to "-drive"

Using "cache=off" with "-drive" will open the disk image file using "O_DIRECT".

By default, "cache" is set to "on" to keep original behavior of qemu.

example:

"-drive file=my_disk.qcow2,cache=off"

---
 block-raw.c    |   12 ++++++++++++
 block.c        |    2 +-
 block.h        |    1 +
 hw/fdc.c       |    7 ++++++-
 hw/ide.c       |   18 ++++++++++++++----
 hw/scsi-disk.c |    3 ++-
 hw/sd.c        |   11 ++++++++++-
 osdep.c        |   20 ++++++++++++++++++++
 osdep.h        |    1 +
 vl.c           |   26 +++++++++++++++++++++++---
 10 files changed, 90 insertions(+), 11 deletions(-)
block-raw.c +12 -0
Index: qemu/block-raw.c
===================================================================
@@ -107,6 +107,10 @@ static int raw_open(BlockDriverState *bs
     }
     if (flags & BDRV_O_CREAT)
         open_flags |= O_CREAT | O_TRUNC;
+#ifdef O_DIRECT
+    if (flags & BDRV_O_DIRECT)
+        open_flags |= O_DIRECT;
+#endif
 
     s->type = FTYPE_FILE;
 

@@ -660,6 +664,10 @@ static int hdev_open(BlockDriverState *b
         open_flags |= O_RDONLY;
         bs->read_only = 1;
     }
+#ifdef O_DIRECT
+    if (flags & BDRV_O_DIRECT)
+        open_flags |= O_DIRECT;
+#endif
 
     s->type = FTYPE_FILE;
 #if defined(__linux__)

@@ -981,6 +989,8 @@ static int raw_open(BlockDriverState *bs
 #else
     overlapped = FILE_FLAG_OVERLAPPED;
 #endif
+    if (flags & BDRV_O_DIRECT)
+        overlapped |= FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH;
     s->hfile = CreateFile(filename, access_flags,
                           FILE_SHARE_READ, NULL,
                           create_flags, overlapped, NULL);

@@ -1349,6 +1359,8 @@ static int hdev_open(BlockDriverState *b
 #else
     overlapped = FILE_FLAG_OVERLAPPED;
 #endif
+    if (flags & BDRV_O_DIRECT)
+        overlapped |= FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH;
     s->hfile = CreateFile(filename, access_flags,
                           FILE_SHARE_READ, NULL,
                           create_flags, overlapped, NULL);
block.h +1 -0
Index: qemu/block.h
===================================================================
@@ -44,6 +44,7 @@ typedef struct QEMUSnapshotInfo {
                                      use a disk image format on top of
                                      it (default for
                                      bdrv_file_open()) */
+#define BDRV_O_DIRECT      0x0020
 
 #ifndef QEMU_IMG
 void bdrv_info(void);
vl.c +23 -3
Index: qemu/vl.c
===================================================================
@@ -4851,8 +4851,11 @@ static int drive_init(const char *str, i
     BlockDriverState *bdrv;
     int max_devs;
     int index;
+    int cache;
+    int bdrv_flags;
     char *params[] = { "bus", "unit", "if", "index", "cyls", "heads",
-                       "secs", "trans", "media", "snapshot", "file", NULL };
+                       "secs", "trans", "media", "snapshot", "file",
+                       "cache", NULL };
 
     if (check_params(buf, sizeof(buf), params, str) < 0) {
          fprintf(stderr, "qemu: unknowm parameter '%s' in '%s'\n",

@@ -4866,6 +4869,7 @@ static int drive_init(const char *str, i
     unit_id = -1;
     translation = BIOS_ATA_TRANSLATION_AUTO;
     index = -1;
+    cache = 1;
 
     if (!strcmp(machine->name, "realview") ||
         !strcmp(machine->name, "SS-5") ||

@@ -5005,6 +5009,17 @@ static int drive_init(const char *str, i
 	}
     }
 
+    if (get_param_value(buf, sizeof(buf), "cache", str)) {
+        if (!strcmp(buf, "off"))
+            cache = 0;
+        else if (!strcmp(buf, "on"))
+            cache = 1;
+        else {
+	    fprintf(stderr, "qemu: invalid cache option\n");
+	    return -1;
+        }
+    }
+
     get_param_value(file, sizeof(file), "file", str);
 
     /* compute bus and unit according index */

@@ -5092,8 +5107,12 @@ static int drive_init(const char *str, i
     }
     if (!file[0])
         return 0;
-    if (bdrv_open(bdrv, file, snapshot ? BDRV_O_SNAPSHOT : 0) < 0 ||
-        qemu_key_check(bdrv, file)) {
+    bdrv_flags = 0;
+    if (snapshot)
+        bdrv_flags |= BDRV_O_SNAPSHOT;
+    if (!cache)
+        bdrv_flags |= BDRV_O_DIRECT;
+    if (bdrv_open(bdrv, file, bdrv_flags) < 0 || qemu_key_check(bdrv, file)) {
         fprintf(stderr, "qemu: could not open disk image %s\n",
                         file);
         return -1;

@@ -7440,6 +7459,7 @@ static void help(int exitcode)
            "-cdrom file     use 'file' as IDE cdrom image (cdrom is ide1 master)\n"
 	   "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][index=i]\n"
            "       [,cyls=c,heads=h,secs=s[,trans=t]][snapshot=on|off]\n"
+           "       [cache=on|off]\n"
 	   "                use 'file' as a drive image\n"
            "-mtdblock file  use 'file' as on-board Flash memory image\n"
            "-sd file        use 'file' as SecureDigital card image\n"
hw/ide.c +14 -4
Index: qemu/hw/ide.c
===================================================================
@@ -365,7 +365,7 @@ typedef struct IDEState {
     EndTransferFunc *end_transfer_func;
     uint8_t *data_ptr;
     uint8_t *data_end;
-    uint8_t io_buffer[MAX_MULT_SECTORS*512 + 4];
+    uint8_t *io_buffer;
     QEMUTimer *sector_write_timer; /* only used for win2k install hack */
     uint32_t irq_count; /* counts IRQs when using win2k install hack */
     /* CF-ATA extended error */

@@ -2372,17 +2372,24 @@ struct partition {
 static int guess_disk_lchs(IDEState *s,
                            int *pcylinders, int *pheads, int *psectors)
 {
-    uint8_t buf[512];
+    uint8_t *buf;
     int ret, i, heads, sectors, cylinders;
     struct partition *p;
     uint32_t nr_sects;
 
+    buf = qemu_memalign(512, 512);
+    if (buf == NULL)
+        return -1;
     ret = bdrv_read(s->bs, 0, buf, 1);
-    if (ret < 0)
+    if (ret < 0) {
+        qemu_free(buf);
         return -1;
+    }
     /* test msdos magic */
-    if (buf[510] != 0x55 || buf[511] != 0xaa)
+    if (buf[510] != 0x55 || buf[511] != 0xaa) {
+        qemu_free(buf);
         return -1;
+    }
     for(i = 0; i < 4; i++) {
         p = ((struct partition *)(buf + 0x1be)) + i;
         nr_sects = le32_to_cpu(p->nr_sects);

@@ -2403,9 +2410,11 @@ static int guess_disk_lchs(IDEState *s,
             printf("guessed geometry: LCHS=%d %d %d\n",
                    cylinders, heads, sectors);
 #endif
+            qemu_free(buf);
             return 0;
         }
     }
+    qemu_free(buf);
     return -1;
 }
 

@@ -2420,6 +2429,7 @@ static void ide_init2(IDEState *ide_stat
 
     for(i = 0; i < 2; i++) {
         s = ide_state + i;
+        s->io_buffer = qemu_memalign(512, MAX_MULT_SECTORS*512 + 4);
         if (i == 0)
             s->bs = hd0;
         else
block.c +1 -1
Index: qemu/block.c
===================================================================
@@ -380,7 +380,7 @@ int bdrv_open2(BlockDriverState *bs, con
     /* Note: for compatibility, we open disk image files as RDWR, and
        RDONLY as fallback */
     if (!(flags & BDRV_O_FILE))
-        open_flags = BDRV_O_RDWR;
+        open_flags = BDRV_O_RDWR | (flags & BDRV_O_DIRECT);
     else
         open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
     ret = drv->bdrv_open(bs, filename, open_flags);
hw/scsi-disk.c +2 -1
Index: qemu/hw/scsi-disk.c
===================================================================
@@ -46,7 +46,7 @@ typedef struct SCSIRequest {
     int sector_count;
     /* The amounnt of data in the buffer.  */
     int buf_len;
-    uint8_t dma_buf[SCSI_DMA_BUF_SIZE];
+    uint8_t *dma_buf;
     BlockDriverAIOCB *aiocb;
     struct SCSIRequest *next;
 } SCSIRequest;

@@ -78,6 +78,7 @@ static SCSIRequest *scsi_new_request(SCS
         free_requests = r->next;
     } else {
         r = qemu_malloc(sizeof(SCSIRequest));
+        r->dma_buf = qemu_memalign(512, SCSI_DMA_BUF_SIZE);
     }
     r->dev = s;
     r->tag = tag;
hw/sd.c +10 -1
Index: qemu/hw/sd.c
===================================================================
@@ -1284,10 +1284,17 @@ int sd_do_command(SDState *sd, struct sd
 static void sd_blk_read(BlockDriverState *bdrv,
                 void *data, uint32_t addr, uint32_t len)
 {
-    uint8_t buf[512];
+    uint8_t *buf;
     uint32_t end = addr + len;
 
+    buf = qemu_memalign(512, 512);
+    if (buf == NULL) {
+        printf("sd_blk_read: cannot allocate memory\n");
+        return;
+    }
+
     if (!bdrv || bdrv_read(bdrv, addr >> 9, buf, 1) == -1) {
+        qemu_free(buf);
         printf("sd_blk_read: read error on host side\n");
         return;
     }

@@ -1297,11 +1304,13 @@ static void sd_blk_read(BlockDriverState
 
         if (bdrv_read(bdrv, end >> 9, buf, 1) == -1) {
             printf("sd_blk_read: read error on host side\n");
+            qemu_free(buf);
             return;
         }
         memcpy(data + 512 - (addr & 511), buf, end & 511);
     } else
         memcpy(data, buf + (addr & 511), len);
+    qemu_free(buf);
 }
 
 static void sd_blk_write(BlockDriverState *bdrv,
hw/fdc.c +6 -1
Index: qemu/hw/fdc.c
===================================================================
@@ -382,7 +382,7 @@ struct fdctrl_t {
     uint8_t cur_drv;
     uint8_t bootsel;
     /* Command FIFO */
-    uint8_t fifo[FD_SECTOR_LEN];
+    uint8_t *fifo;
     uint32_t data_pos;
     uint32_t data_len;
     uint8_t data_state;

@@ -598,6 +598,11 @@ fdctrl_t *fdctrl_init (qemu_irq irq, int
     fdctrl = qemu_mallocz(sizeof(fdctrl_t));
     if (!fdctrl)
         return NULL;
+    fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
+    if (fdctrl->fifo == NULL) {
+        qemu_free(fdctrl);
+        return NULL;
+    }
     fdctrl->result_timer = qemu_new_timer(vm_clock,
                                           fdctrl_result_timer, fdctrl);
osdep.c +20 -0
Index: qemu/osdep.c
===================================================================
@@ -60,6 +60,10 @@ void *qemu_malloc(size_t size)
 }
 
 #if defined(_WIN32)
+void *qemu_memalign(size_t alignment, size_t size)
+{
+    return VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
+}
 
 void *qemu_vmalloc(size_t size)
 {

@@ -171,6 +175,22 @@ static void kqemu_vfree(void *ptr)
 
 #endif
 
+void *qemu_memalign(size_t alignment, size_t size)
+{
+#if defined(_POSIX_C_SOURCE)
+    int ret;
+    void *ptr;
+    ret = posix_memalign(&ptr, alignment, size);
+    if (ret != 0)
+        return NULL;
+    return ptr;
+#elif defined(_BSD)
+    return valloc(size);
+#else
+    return memalign(alignment, size);
+#endif
+}
+
 /* alloc shared memory pages */
 void *qemu_vmalloc(size_t size)
 {
osdep.h +1 -0
Index: qemu/osdep.h
===================================================================
@@ -48,6 +48,7 @@ void *qemu_mallocz(size_t size);
 void qemu_free(void *ptr);
 char *qemu_strdup(const char *str);
 
+void *qemu_memalign(size_t alignment, size_t size);
 void *qemu_vmalloc(size_t size);
 void qemu_vfree(void *ptr);
author
#2 lore raw

PATCH v2 2/2 Direct IDE I/O

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2 
 block_int.h |    1 
 cpu-all.h   |    1 
 exec.c      |   19 ++++++
 hw/ide.c    |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 vl.c        |    1 
 7 files changed, 204 insertions(+), 6 deletions(-)
block.c +10 -0
Index: qemu/block.c
===================================================================
@@ -758,6 +758,11 @@ void bdrv_set_translation_hint(BlockDriv
     bs->translation = translation;
 }
 
+void bdrv_set_cache_hint(BlockDriverState *bs, int cache)
+{
+    bs->cache = cache;
+}
+
 void bdrv_get_geometry_hint(BlockDriverState *bs,
                             int *pcyls, int *pheads, int *psecs)
 {

@@ -786,6 +791,11 @@ int bdrv_is_read_only(BlockDriverState *
     return bs->read_only;
 }
 
+int bdrv_is_cached(BlockDriverState *bs)
+{
+    return bs->cache;
+}
+
 /* XXX: no longer used */
 void bdrv_set_change_cb(BlockDriverState *bs,
                         void (*change_cb)(void *opaque), void *opaque)
block.h +2 -0
Index: qemu/block.h
===================================================================
@@ -113,6 +113,7 @@ void bdrv_set_geometry_hint(BlockDriverS
                             int cyls, int heads, int secs);
 void bdrv_set_type_hint(BlockDriverState *bs, int type);
 void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
+void bdrv_set_cache_hint(BlockDriverState *bs, int cache);
 void bdrv_get_geometry_hint(BlockDriverState *bs,
                             int *pcyls, int *pheads, int *psecs);
 int bdrv_get_type_hint(BlockDriverState *bs);

@@ -120,6 +121,7 @@ int bdrv_get_translation_hint(BlockDrive
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
+int bdrv_is_cached(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 int bdrv_is_locked(BlockDriverState *bs);
 void bdrv_set_locked(BlockDriverState *bs, int locked);
block_int.h +1 -0
Index: qemu/block_int.h
===================================================================
@@ -124,6 +124,7 @@ struct BlockDriverState {
        drivers. They are not used by the block driver */
     int cyls, heads, secs, translation;
     int type;
+    int cache;
     char device_name[32];
     BlockDriverState *next;
 };
vl.c +1 -0
Index: qemu/vl.c
===================================================================
@@ -5112,6 +5112,7 @@ static int drive_init(const char *str, i
         bdrv_flags |= BDRV_O_SNAPSHOT;
     if (!cache)
         bdrv_flags |= BDRV_O_DIRECT;
+    bdrv_set_cache_hint(bdrv, cache);
     if (bdrv_open(bdrv, file, bdrv_flags) < 0 || qemu_key_check(bdrv, file)) {
         fprintf(stderr, "qemu: could not open disk image %s\n",
                         file);
hw/ide.c +170 -6

212 lines not on this page · raw message

exec.c +19 -0
Index: qemu/exec.c
===================================================================
@@ -2054,6 +2054,25 @@ void cpu_register_physical_memory(target
     }
 }
 
+uint8_t * cpu_physical_page_addr(target_phys_addr_t addr)
+{
+    target_phys_addr_t page;
+    unsigned long pd;
+    PhysPageDesc *p;
+    unsigned long addr1;
+
+    page = addr & TARGET_PAGE_MASK;
+    p = phys_page_find(page >> TARGET_PAGE_BITS);
+    if (!p)
+       return (uint8_t*)-1;
+
+    pd = p->phys_offset;
+
+    addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
+
+    return phys_ram_base + addr1;
+}
+
 /* XXX: temporary until new memory mapping API */
 uint32_t cpu_get_physical_page_desc(target_phys_addr_t addr)
 {
cpu-all.h +1 -0
Index: qemu/cpu-all.h
===================================================================
@@ -837,6 +837,7 @@ int cpu_register_io_memory(int io_index,
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index);
 CPUReadMemoryFunc **cpu_get_io_memory_read(int io_index);
 
+extern uint8_t * cpu_physical_page_addr(target_phys_addr_t addr);
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write);
 static inline void cpu_physical_memory_read(target_phys_addr_t addr,
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 11 lines · Laurent Vivier

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2 
 block_int.h |    1 
 cpu-all.h   |    1 
 exec.c      |   19 ++++++
 hw/ide.c    |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 vl.c        |    1 
 7 files changed, 204 insertions(+), 6 deletions(-)

What's the use of keeping the buffered case ?

Fabrice.

author
#4 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 13 lines · Fabrice Bellard
Quoted 11 lines · Laurent Vivier

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2 
 block_int.h |    1 
 cpu-all.h   |    1 
 exec.c      |   19 ++++++
 hw/ide.c    |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 vl.c        |    1 
 7 files changed, 204 insertions(+), 6 deletions(-)
Quoted 1 line

What's the use of keeping the buffered case ?

Well, I don't like to remove code written by others... and I don't want to break something.

But if you think I should remove the buffered case, I can.

BTW, do you think I should enable "cache=off" by default ? Or even remove the option from the command line and always use O_DIRECT ?

Regards,
Laurent

Signature · 3 lines
------------- [email protected]  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke
#5 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Am 03.12.2007 um 11:30 schrieb Laurent Vivier:

Quoted 1 line

But if you think I should remove the buffered case, I can.

In doubt, less code is always better. For the unlikely case you broke something badly, there's always the option to take back the patch.

Quoted 1 line

BTW, do you think I should enable "cache=off" by default ?

This would be fine for a transition phase, but likely, the cache=on case gets forgotten to be removed later. So, do it now.

my $ 0.02,
Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/

#6 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 10 lines · Markus Hitter

Am 03.12.2007 um 11:30 schrieb Laurent Vivier:

Quoted 1 line

But if you think I should remove the buffered case, I can.

In doubt, less code is always better. For the unlikely case you broke something badly, there's always the option to take back the patch.

Quoted 1 line

BTW, do you think I should enable "cache=off" by default ?

This would be fine for a transition phase, but likely, the cache=on case gets forgotten to be removed later. So, do it now.

I think host caching is still useful enough to be enabled by default, and provides a significant performance increase in several cases.

- The guest typically has a relatively small quantity of RAM, compared to a modern machine. Allowing the host OS to act as a demand-based L2 cache allows this to be used without having to dedicate excessive quantities of ram to qemu.
- I've seen reports that it significantly speeds up the windows installer.
- Host cache is persistent between multiple qemu runs. f you're doing anything that requires frequent guest reboots (e.g. kernel debugging) this is going to be a huge win.
- You're running a host OS that has limited or no caching (e.g. DOS).

I'd hope that the host OS would have cache use heuristics that would help limit cache pollution.

Paul

#7 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 12 lines · Paul Brook, le Mon 03 Dec 2007 15:39:48 +0000, a écrit :

I think host caching is still useful enough to be enabled by default, and provides a significant performance increase in several cases.

- The guest typically has a relatively small quantity of RAM, compared to a modern machine. Allowing the host OS to act as a demand-based L2 cache allows this to be used without having to dedicate excessive quantities of ram to qemu.
- I've seen reports that it significantly speeds up the windows installer.
- Host cache is persistent between multiple qemu runs. f you're doing anything that requires frequent guest reboots (e.g. kernel debugging) this is going to be a huge win.
- You're running a host OS that has limited or no caching (e.g. DOS).

Yes, and in other cases (e.g. real-production KVM/Xen servers), this is just cache duplication.

Quoted 2 lines

I'd hope that the host OS would have cache use heuristics that would help limit cache pollution.

How could it? It can't detect that the guest also has a buffer/page cache.

Samuel

#8 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Laurent Vivier
Quoted 19 lines · Laurent Vivier
Quoted 14 lines · Fabrice Bellard
Quoted 12 lines · Laurent Vivier

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2 
 block_int.h |    1 
 cpu-all.h   |    1 
 exec.c      |   19 ++++++
 hw/ide.c    |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 vl.c        |    1 
 7 files changed, 204 insertions(+), 6 deletions(-)
Quoted 2 lines

What's the use of keeping the buffered case ?

Quoted 9 lines

Well, I don't like to remove code written by others... and I don't want to break something.

But if you think I should remove the buffered case, I can.

BTW, do you think I should enable "cache=off" by default ? Or even remove the option from the command line and always use O_DIRECT ?

Hi Laurent,

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk. I've observed this myself. Plus the host typically has a much larger page cache then the guest so the second level of caching helps an awful lot.

Regards,

Anthony Liguori

Quoted 3 lines

Regards,
Laurent

#9 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 3 lines · Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Since qemu can use the aio interface, that shouldn't matter.

Samuel

#10 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 6 lines · Samuel Thibault
Quoted 3 lines · Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Since qemu can use the aio interface, that shouldn't matter.

Only if the emulated hardware and guest OS support multiple concurrent commands. IDE supports async operation, but not concurrent commmands. In practice this means you only get full performance if you're using the SCSI emulation.

Paul

#11 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 6 lines · Paul Brook
Quoted 5 lines · Samuel Thibault
Quoted 3 lines · Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Quoted 6 lines
Quoted 1 line

Since qemu can use the aio interface, that shouldn't matter.

Only if the emulated hardware and guest OS support multiple concurrent commands. IDE supports async operation, but not concurrent commmands. In practice this means you only get full performance if you're using the SCSI emulation.

With the IDE emulation, when the emulated "disk write cache" flag is on it may be reasonable to report a write as completed when the AIO is dispatched, without waiting for the AIO to complete.

An IDE flush cache command would wait for all outstanding write AIOs to complete, and then issue a flush cache (fdatasync) to the real device before reporting it has completed.

That's roughly equivalent to what an IDE disk with write caching does, and it would provide exactly the guarantees for safe storage to the real physical medium that a journalling filesystem or database in the guest requires.

If a guest doesn't use journalling with IDE write cache safely (e.g. 2.4 Linux and earler), it can simply turn off the IDE "disk write cache" flag, which is what it has to do on a real physical disk too.

Terminating the qemu process abruptly might cancel some AIOs, but even that is ok, as it's equivalent to pulling the power on a real disk with uncommitted cached writes.

-- Jamie

#12 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 28 lines · Jamie Lokier
Quoted 4 lines · Paul Brook
Quoted 2 lines · Samuel Thibault

Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Since qemu can use the aio interface, that shouldn't matter.

Only if the emulated hardware and guest OS support multiple concurrent commands. IDE supports async operation, but not concurrent commmands. In practice this means you only get full performance if you're using the SCSI emulation.

With the IDE emulation, when the emulated "disk write cache" flag is on it may be reasonable to report a write as completed when the AIO is dispatched, without waiting for the AIO to complete.

An IDE flush cache command would wait for all outstanding write AIOs to complete, and then issue a flush cache (fdatasync) to the real device before reporting it has completed.

That's roughly equivalent to what an IDE disk with write caching does, and it would provide exactly the guarantees for safe storage to the real physical medium that a journalling filesystem or database in the guest requires.

Quoted 9 lines

If a guest doesn't use journalling with IDE write cache safely (e.g. 2.4 Linux and earler), it can simply turn off the IDE "disk write cache" flag, which is what it has to do on a real physical disk too.

Terminating the qemu process abruptly might cancel some AIOs, but even that is ok, as it's equivalent to pulling the power on a real disk with uncommitted cached writes.

Except that in an enterprise environment, you typically have battery backed disk cache. It really doesn't matter though b/c in QEMU today, submitting the request blocks until it's completed anyway (which is nearly instant anyway since I/O is buffered).

Regards,

Anthony Liguori

Quoted 5 lines

-- Jamie

#13 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 13 lines · Anthony Liguori
Quoted 4 lines

With the IDE emulation, when the emulated "disk write cache" flag is on it may be reasonable to report a write as completed when the AIO is dispatched, without waiting for the AIO to complete.

Quoted 8 lines

An IDE flush cache command would wait for all outstanding write AIOs to complete, and then issue a flush cache (fdatasync) to the real device before reporting it has completed.

That's roughly equivalent to what an IDE disk with write caching does, and it would provide exactly the guarantees for safe storage to the real physical medium that a journalling filesystem or database in the guest requires.

Quoted 4 lines

Except that in an enterprise environment, you typically have battery backed disk cache. It really doesn't matter though b/c in QEMU today, submitting the request blocks until it's completed anyway (which is nearly instant anyway since I/O is buffered).

Buffered I/O is less reliable in a sense.

With buffered I/O, if the host crashes, you may lose data that a filesystem or database on the guest reported as committed to applications. That can result, on those rare occasions, in guest journalled filesystem corruption (something that should be impossible), and in database corruption or durability failure.

With direct I/O and write cache emulation (as described), when a guest journalling filesystem or database reports data is committed, it has much the same committment/durability guarantee that the same applications would have running on the host. Namely, the data has reached the disk, and the disk has reported it's committed.

This may matter if you want to run those sort of applications in a guest, which clearly people often do, especially with KVM or Xen.

Anecdote: This is already a problem in some environments. I have a rented virtual machine; it's running UML. The UML disk uses O_SYNC writes (nowadays), because buffered host writes resulted in occasional guest data loss, and journalled filesystem corruption. Unfortunately, this is a performance slowdown, but it's better than occasional corruption. I imagine similar things apply with Qemu machines occasionally.

-- Jamie

#14 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Samuel Thibault
Quoted 7 lines · Samuel Thibault
Quoted 2 lines · Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Since qemu can use the aio interface, that shouldn't matter.

Well, let's separate a few things. QEMU uses posix-aio which uses threads and normal read/write operations. It also limits the number of threads that aio uses to 1 which effectively makes everything synchronous anyway.

But it still doesn't matter. When you issue a write() on an O_DIRECT fd, the write does not complete until the data has made it's way to disk. The guest can still run if you're using O_NONBLOCK but the IDE device will not submit another IO request until you complete the DMA operation.

The SCSI device supports multiple outstanding operations but it's limited to 16 but you'll never see more than one request at a time in QEMU currently because of the limitation to a single thread.

Regards,

Anthony Liguori

Quoted 5 lines

Samuel

author
#15 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 8 lines · Anthony Liguori
Quoted 7 lines · Samuel Thibault
Quoted 4 lines · Anthony Liguori, le Mon 03 Dec 2007 09:54:47 -0600, a écrit :

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk.

Quoted 7 lines
Quoted 2 lines

Since qemu can use the aio interface, that shouldn't matter.

Well, let's separate a few things. QEMU uses posix-aio which uses threads and normal read/write operations. It also limits the number of threads that aio uses to 1 which effectively makes everything synchronous anyway.

Yes, librt is providing posix-aio, and librt coming with GNU libc uses threads.
But if I remember correctly librt coming with RHEL uses a mix of threads and linux kernel AIO (you can have a look to the .srpm of libc).

There is also the libaio I wrote some years ago (with Sébastien Dugué) which is purely linux kernel AIO (but kernel patches were never included because of Zach Brown Asynchronous System Call work)

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

Quoted 22 lines

But it still doesn't matter. When you issue a write() on an O_DIRECT fd, the write does not complete until the data has made it's way to disk. The guest can still run if you're using O_NONBLOCK but the IDE device will not submit another IO request until you complete the DMA operation.

The SCSI device supports multiple outstanding operations but it's limited to 16 but you'll never see more than one request at a time in QEMU currently because of the limitation to a single thread.

Regards,

Anthony Liguori

Quoted 5 lines

Samuel

Signature · 3 lines
------------- [email protected]  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke
#16 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 7 lines

Yes, librt is providing posix-aio, and librt coming with GNU libc uses threads.
But if I remember correctly librt coming with RHEL uses a mix of threads and linux kernel AIO (you can have a look to the .srpm of libc).

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

This seems rather pointless, given a user can just use a linux-AIO librt instead.

Paul

#17 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 11 lines · Paul Brook
Quoted 8 lines

Yes, librt is providing posix-aio, and librt coming with GNU libc uses threads.
But if I remember correctly librt coming with RHEL uses a mix of threads and linux kernel AIO (you can have a look to the .srpm of libc).

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

This seems rather pointless, given a user can just use a linux-AIO librt instead.

Not at all. linux-aio is the only interface that allows you to do asynchronous fdsync which simulates a barrier which allows for an ordered queue.

I have a patch that uses linux-aio for the virtio-blk driver I'll be posting tomorrow and I'm extremely happy with the results. In recent kernels, you can use an eventfd interface along with linux-aio so that polling is unnecessary. Along with O_DIRECT and the preadv/pwritev interface, you can make a block backend in userspace that performs just as well as if it were in the kernel.

The posix-aio interface simply doesn't provide a mechanism to do these things.

Regards,

Anthony Liguori

Quoted 5 lines

Paul

#18 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 4 lines · Anthony Liguori

I have a patch that uses linux-aio for the virtio-blk driver I'll be posting tomorrow and I'm extremely happy with the results. In recent kernels, you can use an eventfd interface along with linux-aio so that polling is unnecessary.

Which kernel version is "recent"?

cheers,
Gerd

author
#19 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 7 lines · Gerd Hoffmann
Quoted 4 lines · Anthony Liguori

I have a patch that uses linux-aio for the virtio-blk driver I'll be posting tomorrow and I'm extremely happy with the results. In recent kernels, you can use an eventfd interface along with linux-aio so that polling is unnecessary.

Which kernel version is "recent"?

I think it is 2.6.22 and after

Laurent

Signature · 3 lines
------------- [email protected]  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke
author
#20 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Paul Brook
Quoted 10 lines · Paul Brook
Quoted 7 lines

Yes, librt is providing posix-aio, and librt coming with GNU libc uses threads.
But if I remember correctly librt coming with RHEL uses a mix of threads and linux kernel AIO (you can have a look to the .srpm of libc).

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

This seems rather pointless, given a user can just use a linux-AIO librt instead.

Just a comment: to use linux-aio, file must be opened with O_DIRECT. (it's a good reason to include my patch, isn't it ?)

Laurent

Signature · 3 lines
------------- [email protected]  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke
#21 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Laurent Vivier

Hi,

Quoted 2 lines

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

IMHO it would be a much better idea to kill the aio interface altogether and instead make the block drivers reentrant. Then you can use (multiple) posix threads to run the I/O async if you want.

cheers,
Gerd

#22 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 6 lines · Gerd Hoffmann, le Mon 03 Dec 2007 22:13:07 +0100, a écrit :
Quoted 2 lines

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

IMHO it would be a much better idea to kill the aio interface altogether and instead make the block drivers reentrant. Then you can use (multiple) posix threads to run the I/O async if you want.

Mmm, that will not make my life easier... I'm precisely trying to avoid threads so as to get better throughput.

Samuel

#23 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Gerd Hoffmann
Quoted 11 lines · Gerd Hoffmann

Hi,

Quoted 3 lines

BTW, if everyone thinks it could be a good idea I can port block-raw.c to use linux kernel AIO (without removing POSIX AIO support, of course)

IMHO it would be a much better idea to kill the aio interface altogether and instead make the block drivers reentrant. Then you can use (multiple) posix threads to run the I/O async if you want.

Threads are a poor substitute for a proper AIO interface. linux-aio gives you everything you could possibly want in an interface since it allows you to submit multiple vectored operations in a single syscall, use an fd to signal request completion, complete multiple requests in a single syscall, and inject barriers via fdsync.

Regards,

Anthony Liguori

Quoted 8 lines

cheers,
Gerd

#24 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 9 lines · Anthony Liguori
Quoted 3 lines

IMHO it would be a much better idea to kill the aio interface altogether and instead make the block drivers reentrant. Then you can use (multiple) posix threads to run the I/O async if you want.

Threads are a poor substitute for a proper AIO interface. linux-aio gives you everything you could possibly want in an interface since it allows you to submit multiple vectored operations in a single syscall, use an fd to signal request completion, complete multiple requests in a single syscall, and inject barriers via fdsync.

I still think implementing async i/o at block driver level is the wrong thing to do. You'll end up reinventing the wheel over and over again and add complexity to the block drivers which simply doesn't belong there (or not supporting async I/O for most file formats). Just look at the insane file size of the block driver for the simplest possible disk
format: block-raw.c. It will become even worse when adding a linux-specific aio variant.

In contrast: Making the disk drivers reentrant should be easy for most of them. For the raw driver it should be just using pread/pwrite syscalls instead of lseek + read/write (also saves a syscall along the way, yea!). Others probably need an additional lock for metadata updates. With that in place you can easily implement async I/O via threads one layer above, and only once, in block.c.

IMHO the only alternative to that scheme would be to turn the block drivers in some kind of remapping drivers for the various file formats which don't actually perform the I/O. Then you can handle the actual I/O in a generic way using whatever API is available, be it posix-aio, linux-aio or slow-sync-io.

cheers,
Gerd

#25 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 21 lines · Gerd Hoffmann
Quoted 10 lines · Anthony Liguori
Quoted 4 lines

IMHO it would be a much better idea to kill the aio interface altogether and instead make the block drivers reentrant. Then you can use (multiple) posix threads to run the I/O async if you want.

Threads are a poor substitute for a proper AIO interface. linux-aio gives you everything you could possibly want in an interface since it allows you to submit multiple vectored operations in a single syscall, use an fd to signal request completion, complete multiple requests in a single syscall, and inject barriers via fdsync.

I still think implementing async i/o at block driver level is the wrong thing to do. You'll end up reinventing the wheel over and over again and add complexity to the block drivers which simply doesn't belong there (or not supporting async I/O for most file formats). Just look at the insane file size of the block driver for the simplest possible disk
format: block-raw.c. It will become even worse when adding a linux-specific aio variant.

Quoted 7 lines

In contrast: Making the disk drivers reentrant should be easy for most of them. For the raw driver it should be just using pread/pwrite syscalls instead of lseek + read/write (also saves a syscall along the way, yea!). Others probably need an additional lock for metadata updates. With that in place you can easily implement async I/O via threads one layer above, and only once, in block.c.

I really want to use readv/writev though. With virtio, we get a scatter/gather list for each IO request.

Once I post the virtio-blk driver, I'll follow up a little later with some refactoring of the block device layers. I think it can be made much simpler while still remaining asynchronous.

Quoted 6 lines

IMHO the only alternative to that scheme would be to turn the block drivers in some kind of remapping drivers for the various file formats which don't actually perform the I/O. Then you can handle the actual I/O in a generic way using whatever API is available, be it posix-aio, linux-aio or slow-sync-io.

That's part of my plan.

Regards,

Anthony Liguori

Quoted 6 lines

cheers,
Gerd

#26 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 1 line · Anthony Liguori

Gerd Hoffmann wrote:

Hi,

Quoted 2 lines

I really want to use readv/writev though. With virtio, we get a scatter/gather list for each IO request.

Yep, I've also missed pwritev (or whatever that syscall would be named).

Quoted 11 lines

Once I post the virtio-blk driver, I'll follow up a little later with some refactoring of the block device layers. I think it can be made much simpler while still remaining asynchronous.

Quoted 5 lines

IMHO the only alternative to that scheme would be to turn the block drivers in some kind of remapping drivers for the various file formats which don't actually perform the I/O. Then you can handle the actual I/O in a generic way using whatever API is available, be it posix-aio, linux-aio or slow-sync-io.

That's part of my plan.

Oh, cool. Can you also turn them into a sane shared library while being at it? The current approach to compile it once for qemu and once for qemu-img with -DQEMU_TOOL isn't that great. But if you factor out the actual I/O the block-raw.c code should have no need to mess with qemu internals any more and become much cleaner and simpler ...

cheers,
Gerd

#27 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

Quoted 25 lines · Gerd Hoffmann
Quoted 2 lines · Anthony Liguori

Gerd Hoffmann wrote:

Hi,

Quoted 3 lines

I really want to use readv/writev though. With virtio, we get a scatter/gather list for each IO request.

Yep, I've also missed pwritev (or whatever that syscall would be named).

Quoted 11 lines

Once I post the virtio-blk driver, I'll follow up a little later with some refactoring of the block device layers. I think it can be made much simpler while still remaining asynchronous.

Quoted 6 lines

IMHO the only alternative to that scheme would be to turn the block drivers in some kind of remapping drivers for the various file formats which don't actually perform the I/O. Then you can handle the actual I/O in a generic way using whatever API is available, be it posix-aio, linux-aio or slow-sync-io.

Quoted 9 lines
Quoted 2 lines

That's part of my plan.

Oh, cool. Can you also turn them into a sane shared library while being at it? The current approach to compile it once for qemu and once for qemu-img with -DQEMU_TOOL isn't that great. But if you factor out the actual I/O the block-raw.c code should have no need to mess with qemu internals any more and become much cleaner and simpler ...

Yeah, it is definitely something that should be turned into a shared library. I don't think I'll attempt that at first but I do agree it's the right direction to move toward.

Regards,

Anthony Liguori

Quoted 7 lines

cheers,
Gerd

#28 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Anthony Liguori
Quoted 4 lines

Well, let's separate a few things. QEMU uses posix-aio which uses threads and normal read/write operations. It also limits the number of threads that aio uses to 1 which effectively makes everything synchronous anyway.

This is a bug. Allegedly this is to workaround an old broken glibc, so we should probably make it conditional on old glibc.

Paul

author
#29 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Anthony Liguori
Quoted 23 lines · Anthony Liguori
Quoted 22 lines · Laurent Vivier
Quoted 14 lines · Fabrice Bellard
Quoted 12 lines · Laurent Vivier

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2 
 block_int.h |    1 
 cpu-all.h   |    1 
 exec.c      |   19 ++++++
 hw/ide.c    |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 vl.c        |    1 
 7 files changed, 204 insertions(+), 6 deletions(-)
Quoted 2 lines

What's the use of keeping the buffered case ?

Well, I don't like to remove code written by others... and I don't want to break something.

Quoted 8 lines
Quoted 6 lines

But if you think I should remove the buffered case, I can.

BTW, do you think I should enable "cache=off" by default ? Or even remove the option from the command line and always use O_DIRECT ?

Hi Laurent,

Hi Anthony,

Quoted 5 lines

Have you done any performance testing? Buffered IO should absolutely beat direct IO simply because buffered IO allows writes to complete before they actually hit disk. I've observed this myself. Plus the host typically has a much larger page cache then the guest so the second level of caching helps an awful lot.

I don't have real benchmarks. I just saw some improvements with dbench (which is not a good benchmark, I know...)

Direct I/O can be good in some cases (because it avoids multiple copies) and good in others (because it avoids disk access, and as you say it doesn't wait I/O completion).

But there are at least two other good reasons to use it:

- reliability: by avoiding cache we improve probability of data are on disk (and the ordering of I/O). And as you say, as we wait write completion, we are sure data have been written.

- isolation: it allows to avoid to pollute host cache with guest data (and if we have several guests, it avoids to have performance impact at the cache level between guests).

But there is no perfect solution, it's why I think it's good thing to let the choice to the user.

Laurent
-

Signature · 3 lines
------------- [email protected]  --------------
       "Any sufficiently advanced technology is
  indistinguishable from magic." - Arthur C. Clarke
#30 lore raw

Re: [Qemu-devel] [PATCH 2/2 v2] Direct IDE I/O

In reply to Fabrice Bellard

Hi,

Quoted 11 lines · Fabrice Bellard
Quoted 9 lines · Laurent Vivier

This patch enhances the "-drive ,cache=off" mode with IDE drive emulation by removing the buffer used in the IDE emulation.

---
 block.c     |   10 +++
 block.h     |    2  block_int.h |    1  cpu-all.h   |    1  exec.c      |
19 ++++++
 hw/ide.c    |  176

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
vl.c | 1 7 files changed, 204 insertions(+), 6 deletions(-)

Quoted 1 line

What's the use of keeping the buffered case ?

AFAICT if your guest is DOS without a disk caching driver, you do not really want to use O_DIRECT.

Ciao,
Dscho