[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Do we need CONFIG_AIO?
|
From: |
Christoph Hellwig |
|
Subject: |
Re: [Qemu-devel] Do we need CONFIG_AIO? |
|
Date: |
Tue, 26 May 2009 10:19:42 +0200 |
|
User-agent: |
Mutt/1.3.28i |
On Tue, May 26, 2009 at 02:58:09AM -0500, Anthony Liguori wrote:
> Threads are not going to be able to remain an optional dependency
> forever. I would be willing to merge something to remove CONFIG_AIO
> although I expect that there will be some fall out. Some people are
> using --disable-aio today for either historic reasons or because there
> are weird bugs with AIO enabled.
Btw, this is the example patch I had in mind. Also when finishing up
the configure bits I noticed DragonflyBSD currently does disable aio
(but not pthreads in general), but that's the only platform I found.
Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c 2009-05-26 10:10:43.787976336 +0200
+++ qemu/block/raw-posix.c 2009-05-26 10:12:03.328027500 +0200
@@ -26,9 +26,7 @@
#include "qemu-char.h"
#include "block_int.h"
#include "module.h"
-#ifdef CONFIG_AIO
#include "posix-aio-compat.h"
-#endif
#ifdef CONFIG_COCOA
#include <paths.h>
@@ -102,7 +100,7 @@
typedef struct BDRVRawState {
int fd;
int type;
- unsigned int lseek_err_cnt;
+ int flags;
int open_flags;
#if defined(__linux__)
/* linux floppy specific */
@@ -111,7 +109,6 @@ typedef struct BDRVRawState {
int fd_got_error;
int fd_media_changed;
#endif
- uint8_t* aligned_buf;
} BDRVRawState;
static int posix_aio_init(void);
@@ -130,8 +127,6 @@ static int raw_open_common(BlockDriverSt
posix_aio_init();
- s->lseek_err_cnt = 0;
-
s->open_flags |= O_BINARY;
if ((flags & BDRV_O_ACCESS) == O_RDWR) {
s->open_flags |= O_RDWR;
@@ -156,15 +151,8 @@ static int raw_open_common(BlockDriverSt
return ret;
}
s->fd = fd;
- s->aligned_buf = NULL;
- if ((flags & BDRV_O_NOCACHE)) {
- s->aligned_buf = qemu_blockalign(bs, ALIGNED_BUFFER_SIZE);
- if (s->aligned_buf == NULL) {
- ret = -errno;
- close(fd);
- return ret;
- }
- }
+ s->flags = flags;
+
return 0;
}
@@ -196,282 +184,6 @@ static int raw_open(BlockDriverState *bs
#endif
*/
-/*
- * offset and count are in bytes, but must be multiples of 512 for files
- * opened with O_DIRECT. buf must be aligned to 512 bytes then.
- *
- * This function may be called without alignment if the caller ensures
- * that O_DIRECT is not in effect.
- */
-static int raw_pread_aligned(BlockDriverState *bs, int64_t offset,
- uint8_t *buf, int count)
-{
- BDRVRawState *s = bs->opaque;
- int ret;
-
- ret = fd_open(bs);
- if (ret < 0)
- return ret;
-
- if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) {
- ++(s->lseek_err_cnt);
- if(s->lseek_err_cnt <= 10) {
- DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
- "] lseek failed : %d = %s\n",
- s->fd, bs->filename, offset, buf, count,
- bs->total_sectors, errno, strerror(errno));
- }
- return -1;
- }
- s->lseek_err_cnt=0;
-
- ret = read(s->fd, buf, count);
- if (ret == count)
- goto label__raw_read__success;
-
- DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
- "] read failed %d : %d = %s\n",
- s->fd, bs->filename, offset, buf, count,
- bs->total_sectors, ret, errno, strerror(errno));
-
- /* Try harder for CDrom. */
- if (bs->type == BDRV_TYPE_CDROM) {
- lseek(s->fd, offset, SEEK_SET);
- ret = read(s->fd, buf, count);
- if (ret == count)
- goto label__raw_read__success;
- lseek(s->fd, offset, SEEK_SET);
- ret = read(s->fd, buf, count);
- if (ret == count)
- goto label__raw_read__success;
-
- DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
- "] retry read failed %d : %d = %s\n",
- s->fd, bs->filename, offset, buf, count,
- bs->total_sectors, ret, errno, strerror(errno));
- }
-
-label__raw_read__success:
-
- return (ret < 0) ? -errno : ret;
-}
-
-/*
- * offset and count are in bytes, but must be multiples of 512 for files
- * opened with O_DIRECT. buf must be aligned to 512 bytes then.
- *
- * This function may be called without alignment if the caller ensures
- * that O_DIRECT is not in effect.
- */
-static int raw_pwrite_aligned(BlockDriverState *bs, int64_t offset,
- const uint8_t *buf, int count)
-{
- BDRVRawState *s = bs->opaque;
- int ret;
-
- ret = fd_open(bs);
- if (ret < 0)
- return -errno;
-
- if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) {
- ++(s->lseek_err_cnt);
- if(s->lseek_err_cnt) {
- DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%"
- PRId64 "] lseek failed : %d = %s\n",
- s->fd, bs->filename, offset, buf, count,
- bs->total_sectors, errno, strerror(errno));
- }
- return -EIO;
- }
- s->lseek_err_cnt = 0;
-
- ret = write(s->fd, buf, count);
- if (ret == count)
- goto label__raw_write__success;
-
- DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%" PRId64
- "] write failed %d : %d = %s\n",
- s->fd, bs->filename, offset, buf, count,
- bs->total_sectors, ret, errno, strerror(errno));
-
-label__raw_write__success:
-
- return (ret < 0) ? -errno : ret;
-}
-
-
-/*
- * offset and count are in bytes and possibly not aligned. For files opened
- * with O_DIRECT, necessary alignments are ensured before calling
- * raw_pread_aligned to do the actual read.
- */
-static int raw_pread(BlockDriverState *bs, int64_t offset,
- uint8_t *buf, int count)
-{
- BDRVRawState *s = bs->opaque;
- int size, ret, shift, sum;
-
- sum = 0;
-
- if (s->aligned_buf != NULL) {
-
- if (offset & 0x1ff) {
- /* align offset on a 512 bytes boundary */
-
- shift = offset & 0x1ff;
- size = (shift + count + 0x1ff) & ~0x1ff;
- if (size > ALIGNED_BUFFER_SIZE)
- size = ALIGNED_BUFFER_SIZE;
- ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf, size);
- if (ret < 0)
- return ret;
-
- size = 512 - shift;
- if (size > count)
- size = count;
- memcpy(buf, s->aligned_buf + shift, size);
-
- buf += size;
- offset += size;
- count -= size;
- sum += size;
-
- if (count == 0)
- return sum;
- }
- if (count & 0x1ff || (uintptr_t) buf & 0x1ff) {
-
- /* read on aligned buffer */
-
- while (count) {
-
- size = (count + 0x1ff) & ~0x1ff;
- if (size > ALIGNED_BUFFER_SIZE)
- size = ALIGNED_BUFFER_SIZE;
-
- ret = raw_pread_aligned(bs, offset, s->aligned_buf, size);
- if (ret < 0)
- return ret;
-
- size = ret;
- if (size > count)
- size = count;
-
- memcpy(buf, s->aligned_buf, size);
-
- buf += size;
- offset += size;
- count -= size;
- sum += size;
- }
-
- return sum;
- }
- }
-
- return raw_pread_aligned(bs, offset, buf, count) + sum;
-}
-
-static int raw_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
-
- ret = raw_pread(bs, sector_num * 512, buf, nb_sectors * 512);
- if (ret == (nb_sectors * 512))
- ret = 0;
- return ret;
-}
-
-/*
- * offset and count are in bytes and possibly not aligned. For files opened
- * with O_DIRECT, necessary alignments are ensured before calling
- * raw_pwrite_aligned to do the actual write.
- */
-static int raw_pwrite(BlockDriverState *bs, int64_t offset,
- const uint8_t *buf, int count)
-{
- BDRVRawState *s = bs->opaque;
- int size, ret, shift, sum;
-
- sum = 0;
-
- if (s->aligned_buf != NULL) {
-
- if (offset & 0x1ff) {
- /* align offset on a 512 bytes boundary */
- shift = offset & 0x1ff;
- ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf, 512);
- if (ret < 0)
- return ret;
-
- size = 512 - shift;
- if (size > count)
- size = count;
- memcpy(s->aligned_buf + shift, buf, size);
-
- ret = raw_pwrite_aligned(bs, offset - shift, s->aligned_buf, 512);
- if (ret < 0)
- return ret;
-
- buf += size;
- offset += size;
- count -= size;
- sum += size;
-
- if (count == 0)
- return sum;
- }
- if (count & 0x1ff || (uintptr_t) buf & 0x1ff) {
-
- while ((size = (count & ~0x1ff)) != 0) {
-
- if (size > ALIGNED_BUFFER_SIZE)
- size = ALIGNED_BUFFER_SIZE;
-
- memcpy(s->aligned_buf, buf, size);
-
- ret = raw_pwrite_aligned(bs, offset, s->aligned_buf, size);
- if (ret < 0)
- return ret;
-
- buf += ret;
- offset += ret;
- count -= ret;
- sum += ret;
- }
- /* here, count < 512 because (count & ~0x1ff) == 0 */
- if (count) {
- ret = raw_pread_aligned(bs, offset, s->aligned_buf, 512);
- if (ret < 0)
- return ret;
- memcpy(s->aligned_buf, buf, count);
-
- ret = raw_pwrite_aligned(bs, offset, s->aligned_buf, 512);
- if (ret < 0)
- return ret;
- if (count < ret)
- ret = count;
-
- sum += ret;
- }
- return sum;
- }
- }
- return raw_pwrite_aligned(bs, offset, buf, count) + sum;
-}
-
-static int raw_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
-{
- int ret;
- ret = raw_pwrite(bs, sector_num * 512, buf, nb_sectors * 512);
- if (ret == (nb_sectors * 512))
- ret = 0;
- return ret;
-}
-
-#ifdef CONFIG_AIO
/***********************************************************/
/* Unix AIO using POSIX AIO */
@@ -629,7 +341,7 @@ static RawAIOCB *raw_aio_setup(BlockDriv
* boundary. Tell the low level code to ensure that in case it's
* not done yet.
*/
- if (s->aligned_buf)
+ if (s->flags & BDRV_O_NOCACHE)
acb->aiocb.aio_flags |= QEMU_AIO_SECTOR_ALIGNED;
acb->next = posix_aio_state->first_aio;
@@ -702,13 +414,6 @@ static void raw_aio_cancel(BlockDriverAI
raw_aio_remove(acb);
}
-#else /* CONFIG_AIO */
-static int posix_aio_init(void)
-{
- return 0;
-}
-#endif /* CONFIG_AIO */
-
static void raw_close(BlockDriverState *bs)
{
@@ -716,8 +421,6 @@ static void raw_close(BlockDriverState *
if (s->fd >= 0) {
close(s->fd);
s->fd = -1;
- if (s->aligned_buf != NULL)
- qemu_free(s->aligned_buf);
}
}
@@ -866,18 +569,14 @@ static BlockDriver bdrv_raw = {
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
.bdrv_open = raw_open,
- .bdrv_read = raw_read,
- .bdrv_write = raw_write,
.bdrv_close = raw_close,
.bdrv_create = raw_create,
.bdrv_flush = raw_flush,
-#ifdef CONFIG_AIO
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
.bdrv_aio_cancel = raw_aio_cancel,
.aiocb_size = sizeof(RawAIOCB),
-#endif
.bdrv_truncate = raw_truncate,
.bdrv_getlength = raw_getlength,
@@ -978,7 +677,7 @@ static int hdev_open(BlockDriverState *b
#endif
s->type = FTYPE_FILE;
-#if defined(__linux__) && defined(CONFIG_AIO)
+#if defined(__linux__)
if (strstart(filename, "/dev/sg", NULL)) {
bs->sg = 1;
}
@@ -1044,7 +743,6 @@ static int hdev_ioctl(BlockDriverState *
return ioctl(s->fd, req, buf);
}
-#ifdef CONFIG_AIO
static BlockDriverAIOCB *hdev_aio_ioctl(BlockDriverState *bs,
unsigned long int req, void *buf,
BlockDriverCompletionFunc *cb, void *opaque)
@@ -1075,7 +773,6 @@ static BlockDriverAIOCB *hdev_aio_ioctl(
return &acb->common;
}
-#endif
#elif defined(__FreeBSD__)
static int fd_open(BlockDriverState *bs)
@@ -1134,24 +831,18 @@ static BlockDriver bdrv_host_device = {
.bdrv_create = hdev_create,
.bdrv_flush = raw_flush,
-#ifdef CONFIG_AIO
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
.bdrv_aio_cancel = raw_aio_cancel,
.aiocb_size = sizeof(RawAIOCB),
-#endif
- .bdrv_read = raw_read,
- .bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
/* generic scsi device */
#ifdef __linux__
.bdrv_ioctl = hdev_ioctl,
-#ifdef CONFIG_AIO
.bdrv_aio_ioctl = hdev_aio_ioctl,
#endif
-#endif
};
#ifdef __linux__
@@ -1228,15 +919,11 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_create = hdev_create,
.bdrv_flush = raw_flush,
-#ifdef CONFIG_AIO
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
.bdrv_aio_cancel = raw_aio_cancel,
.aiocb_size = sizeof(RawAIOCB),
-#endif
- .bdrv_read = raw_read,
- .bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
/* removable device support */
@@ -1305,15 +992,11 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_create = hdev_create,
.bdrv_flush = raw_flush,
-#ifdef CONFIG_AIO
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
.bdrv_aio_cancel = raw_aio_cancel,
.aiocb_size = sizeof(RawAIOCB),
-#endif
- .bdrv_read = raw_read,
- .bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
/* removable device support */
@@ -1323,9 +1006,7 @@ static BlockDriver bdrv_host_cdrom = {
/* generic scsi device */
.bdrv_ioctl = hdev_ioctl,
-#ifdef CONFIG_AIO
.bdrv_aio_ioctl = hdev_aio_ioctl,
-#endif
};
#endif /* __linux__ */
@@ -1421,15 +1102,11 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_create = hdev_create,
.bdrv_flush = raw_flush,
-#ifdef CONFIG_AIO
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
.bdrv_aio_cancel = raw_aio_cancel,
.aiocb_size = sizeof(RawAIOCB),
-#endif
- .bdrv_read = raw_read,
- .bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
/* removable device support */
Index: qemu/Makefile
===================================================================
--- qemu.orig/Makefile 2009-05-26 10:12:10.201963074 +0200
+++ qemu/Makefile 2009-05-26 10:12:25.209855741 +0200
@@ -74,10 +74,7 @@ BLOCK_OBJS+=nbd.o block.o aio.o
ifdef CONFIG_WIN32
BLOCK_OBJS += block/raw-win32.o
else
-ifdef CONFIG_AIO
-BLOCK_OBJS += posix-aio-compat.o
-endif
-BLOCK_OBJS += block/raw-posix.o
+BLOCK_OBJS += block/raw-posix.o posix-aio-compat.o
endif
ifdef CONFIG_CURL
Index: qemu/configure
===================================================================
--- qemu.orig/configure 2009-05-26 10:13:16.706814350 +0200
+++ qemu/configure 2009-05-26 10:16:16.304814641 +0200
@@ -181,7 +181,6 @@ uname_release=""
curses="yes"
curl="yes"
pthread="yes"
-aio="yes"
io_thread="no"
nptl="yes"
mixemu="no"
@@ -246,7 +245,6 @@ audio_possible_drivers="oss sdl esd pa"
if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
kqemu="yes"
fi
-aio="no"
;;
NetBSD)
bsd="yes"
@@ -481,10 +479,6 @@ for opt do
;;
--enable-mixemu) mixemu="yes"
;;
- --disable-pthread) pthread="no"
- ;;
- --disable-aio) aio="no"
- ;;
--enable-io-thread) io_thread="yes"
;;
--disable-blobs) blobs="no"
@@ -620,8 +614,6 @@ echo " --oss-lib path to
echo " --enable-uname-release=R Return R for uname -r in usermode emulation"
echo " --sparc_cpu=V Build qemu for Sparc architecture v7, v8,
v8plus, v8plusa, v9"
echo " --disable-vde disable support for vde network"
-echo " --disable-pthread disable pthread support"
-echo " --disable-aio disable AIO support"
echo " --enable-io-thread enable IO thread"
echo " --disable-blobs disable installing provided firmware blobs"
echo " --kerneldir=PATH look for kernel includes in PATH"
@@ -1152,21 +1144,22 @@ fi
# pthread probe
PTHREADLIBS=""
-if test "$pthread" = yes; then
- pthread=no
+pthread=no
cat > $TMPC << EOF
#include <pthread.h>
int main(void) { pthread_mutex_t lock; return 0; }
EOF
- if $cc $ARCH_CFLAGS -o $TMPE $PTHREADLIBS $TMPC 2> /dev/null > /dev/null ;
then
- pthread=yes
- PTHREADLIBS="-lpthread"
- fi
+if $cc $ARCH_CFLAGS -o $TMPE $PTHREADLIBS $TMPC 2> /dev/null > /dev/null ; then
+ pthread=yes
+ PTHREADLIBS="-lpthread"
fi
if test "$pthread" = no; then
- aio=no
- io_thread=no
+ echo
+ echo "Error: pthread check failed"
+ echo "Make sure to have the pthread libs and headers installed."
+ echo
+ exit 1
fi
##########################################
@@ -1354,7 +1347,6 @@ echo "Documentation $build_docs"
echo "uname -r $uname_release"
echo "NPTL support $nptl"
echo "vde support $vde"
-echo "AIO support $aio"
echo "IO thread $io_thread"
echo "Install blobs $blobs"
echo "KVM support $kvm"
@@ -1666,10 +1658,6 @@ fi
if test "$xen" = "yes" ; then
echo "XEN_LIBS=-lxenstore -lxenctrl -lxenguest" >> $config_mak
fi
-if test "$aio" = "yes" ; then
- echo "#define CONFIG_AIO 1" >> $config_h
- echo "CONFIG_AIO=yes" >> $config_mak
-fi
if test "$io_thread" = "yes" ; then
echo "CONFIG_IOTHREAD=yes" >> $config_mak
echo "#define CONFIG_IOTHREAD 1" >> $config_h
- Re: [Qemu-devel] Do we need CONFIG_AIO?, (continued)
Re: [Qemu-devel] Do we need CONFIG_AIO?, Todd T. Fries, 2009/05/25
Re: [Qemu-devel] Do we need CONFIG_AIO?, Anthony Liguori, 2009/05/26